From 7f58fa510c5133607a40e313dfdf74edb1d745f4 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 26 Jul 2023 16:28:26 +0530 Subject: [PATCH 01/73] api change Signed-off-by: Rama Chavali --- .../filters/http/composite/v3/composite.proto | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index 08a72e411b9f..6e40fd5af8bd 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package envoy.extensions.filters.http.composite.v3; import "envoy/config/core/v3/extension.proto"; +import "envoy/config/core/v3/config_source.proto"; import "xds/annotations/v3/status.proto"; @@ -35,5 +36,15 @@ message Composite { // Composite match action (see :ref:`matching docs ` for more info on match actions). // This specifies the filter configuration of the filter that the composite filter should delegate filter interactions to. message ExecuteFilterAction { - config.core.v3.TypedExtensionConfig typed_config = 1; + oneof config_type { + // Filter specific configuration which depends on the filter being + // instantiated. See the supported filters for further documentation. + // [#extension-category: envoy.filters.listener,envoy.filters.udp_listener] + config.core.v3.TypedExtensionConfig typed_config = 1; + + // Configuration source specifier for an extension configuration discovery + // service. In case of a failure and without the default configuration, the + // listener closes the connections. + config.core.v3.ExtensionConfigSource config_discovery = 2; + } } From 449137428462885c39406e93b0721e71d3d02feb Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 8 Aug 2023 20:27:27 +0530 Subject: [PATCH 02/73] initial impl Signed-off-by: Rama Chavali --- source/extensions/filters/http/composite/action.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 40fdbe1bf45f..66a4f55fdf79 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -14,6 +14,14 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( const auto& composite_action = MessageUtil::downcastAndValidate< const envoy::extensions::filters::http::composite::v3::ExecuteFilterAction&>( config, validation_visitor); + if (composite_action.has_config_discovery()){ + auto config_discovery = composite_action.config_discovery(); + if context.server_factory_context_.has_value(){ + std::shared_ptr filter_config_provider_manager = Http::FilterChainUtility::createSingletonUpstreamFilterConfigProviderManager( + context.getServerFactoryContext()); + } + + } else { auto& factory = Config::Utility::getAndCheckFactory( @@ -43,7 +51,8 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( return [cb = std::move(callback)]() -> Matcher::ActionPtr { return std::make_unique(cb); }; -} + } +|} REGISTER_FACTORY(ExecuteFilterActionFactory, Matcher::ActionFactory); From 23fe05e19ee99bf4ddd8e3202b97fb6e2ffe3bc5 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 16 Aug 2023 10:10:55 +0530 Subject: [PATCH 03/73] wip Signed-off-by: Rama Chavali --- .../extensions/filters/http/composite/BUILD | 1 + .../filters/http/composite/action.cc | 21 ++++++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/source/extensions/filters/http/composite/BUILD b/source/extensions/filters/http/composite/BUILD index 512c9b054913..bf636545cf4b 100644 --- a/source/extensions/filters/http/composite/BUILD +++ b/source/extensions/filters/http/composite/BUILD @@ -16,6 +16,7 @@ envoy_cc_library( srcs = ["action.cc"], hdrs = ["action.h"], deps = [ + "//source/common/http:filter_chain_helper_lib", "//source/common/http/matching:data_impl_lib", "//source/common/matcher:matcher_lib", "@envoy_api//envoy/extensions/filters/http/composite/v3:pkg_cc_proto", diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 66a4f55fdf79..f32ee7a2cd45 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -1,3 +1,4 @@ +#include "source/common/http/filter_chain_helper.h" #include "source/extensions/filters/http/composite/action.h" namespace Envoy { @@ -16,11 +17,21 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( config, validation_visitor); if (composite_action.has_config_discovery()){ auto config_discovery = composite_action.config_discovery(); - if context.server_factory_context_.has_value(){ - std::shared_ptr filter_config_provider_manager = Http::FilterChainUtility::createSingletonUpstreamFilterConfigProviderManager( - context.getServerFactoryContext()); + if (context.server_factory_context_.has_value()){ + Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); + Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); + + std::shared_ptr filter_config_provider_manager = Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( + server_factory_context); + Envoy::Http::FilterFactoryCb callback = nullptr; + std::unique_ptr> provider = filter_config_provider_manager->createDynamicFilterConfigProvider( + config_discovery, "name", server_factory_context, factory_context, server_factory_context.clusterManager(), + false, "http", nullptr); + return [cb = provider->config().value()]() -> Matcher::ActionPtr { + return std::make_unique(cb); + }; } - + } else { auto& factory = @@ -52,7 +63,7 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( return std::make_unique(cb); }; } -|} +} REGISTER_FACTORY(ExecuteFilterActionFactory, Matcher::ActionFactory); From 27b1279aa1172987dc374fc371e578f849e7a9d3 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Mon, 28 Aug 2023 12:11:19 +0530 Subject: [PATCH 04/73] add basic implementation Signed-off-by: Rama Chavali --- .../filters/http/composite/v3/composite.proto | 19 ++++- .../filters/http/composite/action.cc | 85 ++++++++++--------- 2 files changed, 59 insertions(+), 45 deletions(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index 6e40fd5af8bd..6427747d3299 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -4,6 +4,7 @@ package envoy.extensions.filters.http.composite.v3; import "envoy/config/core/v3/extension.proto"; import "envoy/config/core/v3/config_source.proto"; +import "validate/validate.proto"; import "xds/annotations/v3/status.proto"; @@ -33,6 +34,17 @@ message Composite { option (xds.annotations.v3.message_status).work_in_progress = true; } +// Configuration for an extension configuration discovery service with name. +message DynamicConfig { + // The name of the extension configuration. It also serves as a resource name in ExtensionConfigDS. + string name = 1 [(validate.rules).string = {min_len: 1}]; + + // Configuration source specifier for an extension configuration discovery + // service. In case of a failure and without the default configuration, the + // listener closes the connections. + config.core.v3.ExtensionConfigSource config_discovery = 2; +} + // Composite match action (see :ref:`matching docs ` for more info on match actions). // This specifies the filter configuration of the filter that the composite filter should delegate filter interactions to. message ExecuteFilterAction { @@ -42,9 +54,8 @@ message ExecuteFilterAction { // [#extension-category: envoy.filters.listener,envoy.filters.udp_listener] config.core.v3.TypedExtensionConfig typed_config = 1; - // Configuration source specifier for an extension configuration discovery - // service. In case of a failure and without the default configuration, the - // listener closes the connections. - config.core.v3.ExtensionConfigSource config_discovery = 2; + // Dynamic configuration of filter obtained via extension configuration discovery + // service . + DynamicConfig dynamic_config = 2; } } diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index f32ee7a2cd45..cfd025fec6a4 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -1,6 +1,7 @@ -#include "source/common/http/filter_chain_helper.h" #include "source/extensions/filters/http/composite/action.h" +#include "source/common/http/filter_chain_helper.h" + namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -15,54 +16,56 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( const auto& composite_action = MessageUtil::downcastAndValidate< const envoy::extensions::filters::http::composite::v3::ExecuteFilterAction&>( config, validation_visitor); - if (composite_action.has_config_discovery()){ - auto config_discovery = composite_action.config_discovery(); - if (context.server_factory_context_.has_value()){ - Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); - Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); + // Process dynamic filter configuration and setup extension configuration discovery service. + if (composite_action.has_dynamic_config()) { + auto config_discovery = composite_action.dynamic_config().config_discovery(); + Server::Configuration::ServerFactoryContext& server_factory_context = + context.server_factory_context_.value(); + Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); - std::shared_ptr filter_config_provider_manager = Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( + std::shared_ptr filter_config_provider_manager = + Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( server_factory_context); - Envoy::Http::FilterFactoryCb callback = nullptr; - std::unique_ptr> provider = filter_config_provider_manager->createDynamicFilterConfigProvider( - config_discovery, "name", server_factory_context, factory_context, server_factory_context.clusterManager(), - false, "http", nullptr); - return [cb = provider->config().value()]() -> Matcher::ActionPtr { - return std::make_unique(cb); - }; - } + Envoy::Http::FilterFactoryCb callback = nullptr; + std::unique_ptr< + Envoy::Filter::DynamicFilterConfigProvider> + provider = filter_config_provider_manager->createDynamicFilterConfigProvider( + config_discovery, composite_action.dynamic_config().name(), server_factory_context, + factory_context, server_factory_context.clusterManager(), false, "http", nullptr); + return [cb = provider->config().value().get().factory_cb]() -> Matcher::ActionPtr { + return std::make_unique(cb); + }; + } else { + // Static filter configuration. + auto& factory = + Config::Utility::getAndCheckFactory( + composite_action.typed_config()); + ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig( + composite_action.typed_config().typed_config(), validation_visitor, factory); - } else { + Envoy::Http::FilterFactoryCb callback = nullptr; - auto& factory = - Config::Utility::getAndCheckFactory( - composite_action.typed_config()); - ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig( - composite_action.typed_config().typed_config(), validation_visitor, factory); + // First, try to create the filter factory creation function from factory context (if exists). + if (context.factory_context_.has_value()) { + callback = factory.createFilterFactoryFromProto(*message, context.stat_prefix_, + context.factory_context_.value()); + } - Envoy::Http::FilterFactoryCb callback = nullptr; + // If above failed, try to create the filter factory creation function from server factory + // context (if exists). + if (callback == nullptr && context.server_factory_context_.has_value()) { + callback = factory.createFilterFactoryFromProtoWithServerContext( + *message, context.stat_prefix_, context.server_factory_context_.value()); + } - // First, try to create the filter factory creation function from factory context (if exists). - if (context.factory_context_.has_value()) { - callback = factory.createFilterFactoryFromProto(*message, context.stat_prefix_, - context.factory_context_.value()); - } + if (callback == nullptr) { + throw EnvoyException("Failed to get filter factory creation function"); + } - // If above failed, try to create the filter factory creation function from server factory - // context (if exists). - if (callback == nullptr && context.server_factory_context_.has_value()) { - callback = factory.createFilterFactoryFromProtoWithServerContext( - *message, context.stat_prefix_, context.server_factory_context_.value()); + return [cb = std::move(callback)]() -> Matcher::ActionPtr { + return std::make_unique(cb); + }; } - - if (callback == nullptr) { - throw EnvoyException("Failed to get filter factory creation function"); - } - - return [cb = std::move(callback)]() -> Matcher::ActionPtr { - return std::make_unique(cb); - }; - } } REGISTER_FACTORY(ExecuteFilterActionFactory, From 07b5e9c161977e516d87075e5110aedea4d97364 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Mon, 28 Aug 2023 21:52:56 +0530 Subject: [PATCH 05/73] manual tested Signed-off-by: Rama Chavali --- .../filters/http/composite/action.cc | 22 +++++++++---------- .../filters/http/composite/action.h | 6 +++++ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index cfd025fec6a4..bcaf50f1c407 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -1,7 +1,5 @@ #include "source/extensions/filters/http/composite/action.h" -#include "source/common/http/filter_chain_helper.h" - namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -22,18 +20,20 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); - std::shared_ptr filter_config_provider_manager = Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( server_factory_context); - Envoy::Http::FilterFactoryCb callback = nullptr; - std::unique_ptr< - Envoy::Filter::DynamicFilterConfigProvider> - provider = filter_config_provider_manager->createDynamicFilterConfigProvider( - config_discovery, composite_action.dynamic_config().name(), server_factory_context, - factory_context, server_factory_context.clusterManager(), false, "http", nullptr); - return [cb = provider->config().value().get().factory_cb]() -> Matcher::ActionPtr { - return std::make_unique(cb); + provider_ = filter_config_provider_manager->createDynamicFilterConfigProvider( + config_discovery, composite_action.dynamic_config().name(), server_factory_context, + factory_context, server_factory_context.clusterManager(), false, "http", nullptr); + return [this]() -> Matcher::ActionPtr { + auto config_value = provider_->config(); + if (config_value.has_value()) { + auto factory_cb = config_value.value().get().factory_cb; + return std::make_unique(factory_cb); + } + // TODO(ramaraochavali): Add a missing config filter. + return nullptr; }; } else { // Static filter configuration. diff --git a/source/extensions/filters/http/composite/action.h b/source/extensions/filters/http/composite/action.h index 725eadada76d..6d2319b7ea19 100644 --- a/source/extensions/filters/http/composite/action.h +++ b/source/extensions/filters/http/composite/action.h @@ -2,6 +2,7 @@ #include "envoy/extensions/filters/http/composite/v3/composite.pb.validate.h" +#include "source/common/http/filter_chain_helper.h" #include "source/common/http/matching/data_impl.h" #include "source/common/matcher/matcher.h" @@ -36,6 +37,11 @@ class ExecuteFilterActionFactory ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique(); } + +private: + std::unique_ptr< + Envoy::Filter::DynamicFilterConfigProvider> + provider_; }; DECLARE_FACTORY(ExecuteFilterActionFactory); From 5f1e3f4e468658cd649e9dc48b6d67571496d72d Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 29 Aug 2023 20:27:57 +0530 Subject: [PATCH 06/73] add integration test Signed-off-by: Rama Chavali --- .../filters/http/composite/v3/composite.proto | 4 +- .../filters/http/composite/action.cc | 9 ++- test/extensions/filters/http/composite/BUILD | 2 + .../composite_filter_integration_test.cc | 70 +++++++++++++++++++ 4 files changed, 78 insertions(+), 7 deletions(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index 6427747d3299..1e87a527de7e 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -2,13 +2,13 @@ syntax = "proto3"; package envoy.extensions.filters.http.composite.v3; -import "envoy/config/core/v3/extension.proto"; import "envoy/config/core/v3/config_source.proto"; -import "validate/validate.proto"; +import "envoy/config/core/v3/extension.proto"; import "xds/annotations/v3/status.proto"; import "udpa/annotations/status.proto"; +import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.http.composite.v3"; option java_outer_classname = "CompositeProto"; diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index bcaf50f1c407..b1b91541691f 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -28,12 +28,11 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( factory_context, server_factory_context.clusterManager(), false, "http", nullptr); return [this]() -> Matcher::ActionPtr { auto config_value = provider_->config(); - if (config_value.has_value()) { - auto factory_cb = config_value.value().get().factory_cb; - return std::make_unique(factory_cb); + if (!config_value.has_value()) { + throw EnvoyException("Failed to get dynamic config for filter"); } - // TODO(ramaraochavali): Add a missing config filter. - return nullptr; + auto factory_cb = config_value.value().get().factory_cb; + return std::make_unique(factory_cb); }; } else { // Static filter configuration. diff --git a/test/extensions/filters/http/composite/BUILD b/test/extensions/filters/http/composite/BUILD index f846deb7b778..fc7eaedab7c5 100644 --- a/test/extensions/filters/http/composite/BUILD +++ b/test/extensions/filters/http/composite/BUILD @@ -34,6 +34,7 @@ envoy_extension_cc_test( "//source/common/http/match_delegate:config", "//source/extensions/filters/http/composite:config", "//source/extensions/filters/http/composite:filter_lib", + "//source/extensions/filters/http/lua:lua_filter_lib", "//source/extensions/matching/http/cel_input:cel_input_lib", "//source/extensions/matching/input_matchers/cel_matcher:cel_matcher_lib", "//source/extensions/matching/input_matchers/cel_matcher:config", @@ -46,6 +47,7 @@ envoy_extension_cc_test( "//test/integration/filters:set_response_code_filter_lib", "//test/proto:helloworld_proto_cc_proto", "@com_github_cncf_udpa//xds/type/matcher/v3:pkg_cc_proto", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/common/matching/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/http/composite/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index fe75b1a45213..54be5727ebe4 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -119,6 +119,53 @@ class CompositeFilterIntegrationTest : public testing::TestWithParammakeRequestWithBody(default_request_headers_, 1024); + waitForNextUpstreamRequest(); + + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_THAT(response->headers(), Http::HttpStatusIs("200")); + } + + { + auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_THAT(response->headers(), Http::HeaderValueOf("lua-invoked", "true")); + } +} + // Verifies function of the per-route config in the ExtensionWithMatcher class. TEST_P(CompositeFilterIntegrationTest, TestPerRoute) { prependCompositeFilter(); From fc62678561b62e9128988533aab44046ac96e5c4 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 30 Aug 2023 12:26:11 +0530 Subject: [PATCH 07/73] fix integration test Signed-off-by: Rama Chavali --- .../filters/http/composite/action.cc | 2 +- test/extensions/filters/http/composite/BUILD | 2 - .../composite_filter_integration_test.cc | 62 +++++++++---------- 3 files changed, 31 insertions(+), 35 deletions(-) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index b1b91541691f..34fb471e3624 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -32,7 +32,7 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( throw EnvoyException("Failed to get dynamic config for filter"); } auto factory_cb = config_value.value().get().factory_cb; - return std::make_unique(factory_cb); + return std::make_unique(std::move(factory_cb)); }; } else { // Static filter configuration. diff --git a/test/extensions/filters/http/composite/BUILD b/test/extensions/filters/http/composite/BUILD index fc7eaedab7c5..f846deb7b778 100644 --- a/test/extensions/filters/http/composite/BUILD +++ b/test/extensions/filters/http/composite/BUILD @@ -34,7 +34,6 @@ envoy_extension_cc_test( "//source/common/http/match_delegate:config", "//source/extensions/filters/http/composite:config", "//source/extensions/filters/http/composite:filter_lib", - "//source/extensions/filters/http/lua:lua_filter_lib", "//source/extensions/matching/http/cel_input:cel_input_lib", "//source/extensions/matching/input_matchers/cel_matcher:cel_matcher_lib", "//source/extensions/matching/input_matchers/cel_matcher:config", @@ -47,7 +46,6 @@ envoy_extension_cc_test( "//test/integration/filters:set_response_code_filter_lib", "//test/proto:helloworld_proto_cc_proto", "@com_github_cncf_udpa//xds/type/matcher/v3:pkg_cc_proto", - "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/common/matching/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/http/composite/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index 54be5727ebe4..38675fafc959 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -143,26 +143,24 @@ class CompositeFilterIntegrationTest : public testing::TestWithParammakeRequestWithBody(default_request_headers_, 1024); - waitForNextUpstreamRequest(); - - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_THAT(response->headers(), Http::HttpStatusIs("200")); - } - - { - auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024); - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_THAT(response->headers(), Http::HeaderValueOf("lua-invoked", "true")); - } -} +// TEST_P(CompositeFilterIntegrationTest, TestBasicDynamicFilter) { +// prependCompositeDynamicFilter("composite-dynamic"); +// initialize(); +// codec_client_ = makeHttpConnection(lookupPort("http")); + +// { +// auto response = codec_client_->makeRequestWithBody(default_request_headers_, 1024); +// waitForNextUpstreamRequest(); + +// upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); +// ASSERT_TRUE(response->waitForEndStream()); +// EXPECT_THAT(response->headers(), Http::HttpStatusIs("200")); +// } + +// { +// auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024); +// ASSERT_TRUE(response->waitForEndStream()); +// EXPECT_THAT(response->headers(), Http::HttpStatusIs("403")); +// } +// } // Verifies function of the per-route config in the ExtensionWithMatcher class. TEST_P(CompositeFilterIntegrationTest, TestPerRoute) { From 4f1baf4eacf4075ff10933ce71d5d52fdb1988aa Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 30 Aug 2023 14:58:18 +0530 Subject: [PATCH 08/73] fix life time Signed-off-by: Rama Chavali --- source/extensions/filters/http/composite/action.cc | 5 +++-- source/extensions/filters/http/composite/action.h | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 34fb471e3624..5e5c005eb9fe 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -19,11 +19,12 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( auto config_discovery = composite_action.dynamic_config().config_discovery(); Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); + std::cout << "creating singleton manager and provider" << std::endl; Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); - std::shared_ptr filter_config_provider_manager = + filter_config_provider_manager_ = Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( server_factory_context); - provider_ = filter_config_provider_manager->createDynamicFilterConfigProvider( + provider_ = filter_config_provider_manager_->createDynamicFilterConfigProvider( config_discovery, composite_action.dynamic_config().name(), server_factory_context, factory_context, server_factory_context.clusterManager(), false, "http", nullptr); return [this]() -> Matcher::ActionPtr { diff --git a/source/extensions/filters/http/composite/action.h b/source/extensions/filters/http/composite/action.h index 6d2319b7ea19..e6507390ecb0 100644 --- a/source/extensions/filters/http/composite/action.h +++ b/source/extensions/filters/http/composite/action.h @@ -27,6 +27,9 @@ class ExecuteFilterActionFactory : public Logger::Loggable, public Matcher::ActionFactory { public: + ~ExecuteFilterActionFactory() { + std::cout << "execute action factory destructo called..." << std::endl; + } std::string name() const override { return "composite-action"; } Matcher::ActionFactoryCb @@ -42,6 +45,7 @@ class ExecuteFilterActionFactory std::unique_ptr< Envoy::Filter::DynamicFilterConfigProvider> provider_; + std::shared_ptr filter_config_provider_manager_; }; DECLARE_FACTORY(ExecuteFilterActionFactory); From 3de5e18e5550832876d8d47a912d37fe8731097b Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 30 Aug 2023 15:54:54 +0530 Subject: [PATCH 09/73] add debug logs Signed-off-by: Rama Chavali --- source/extensions/filters/http/composite/action.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 5e5c005eb9fe..9dc4e634c1ab 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -24,9 +24,11 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( filter_config_provider_manager_ = Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( server_factory_context); + std::cout << "created filter config provider manager" << std::endl; provider_ = filter_config_provider_manager_->createDynamicFilterConfigProvider( config_discovery, composite_action.dynamic_config().name(), server_factory_context, factory_context, server_factory_context.clusterManager(), false, "http", nullptr); + std::cout << "created dynamic filter config provider" << std::endl; return [this]() -> Matcher::ActionPtr { auto config_value = provider_->config(); if (!config_value.has_value()) { From f176faf68d988659b38a945bd32b928e7bc99664 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 30 Aug 2023 16:15:03 +0530 Subject: [PATCH 10/73] more debug Signed-off-by: Rama Chavali --- source/extensions/filters/http/composite/action.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 9dc4e634c1ab..f2a16d8ee6c6 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -25,6 +25,9 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( server_factory_context); std::cout << "created filter config provider manager" << std::endl; + if (filter_config_provider_manager_ == nullptr) { + throw EnvoyException("Failed to create filter config provider manager"); + } provider_ = filter_config_provider_manager_->createDynamicFilterConfigProvider( config_discovery, composite_action.dynamic_config().name(), server_factory_context, factory_context, server_factory_context.clusterManager(), false, "http", nullptr); From 5562c34ddc1d539e0bc2cb96a25da605459f4159 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 30 Aug 2023 17:31:34 +0530 Subject: [PATCH 11/73] more debug Signed-off-by: Rama Chavali --- source/extensions/filters/http/composite/action.cc | 6 +++--- source/extensions/filters/http/composite/action.h | 5 +---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index f2a16d8ee6c6..0c81ead3d829 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -21,14 +21,14 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( context.server_factory_context_.value(); std::cout << "creating singleton manager and provider" << std::endl; Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); - filter_config_provider_manager_ = + std::shared_ptr filter_config_provider_manager = Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( server_factory_context); std::cout << "created filter config provider manager" << std::endl; - if (filter_config_provider_manager_ == nullptr) { + if (filter_config_provider_manager == nullptr) { throw EnvoyException("Failed to create filter config provider manager"); } - provider_ = filter_config_provider_manager_->createDynamicFilterConfigProvider( + provider_ = filter_config_provider_manager->createDynamicFilterConfigProvider( config_discovery, composite_action.dynamic_config().name(), server_factory_context, factory_context, server_factory_context.clusterManager(), false, "http", nullptr); std::cout << "created dynamic filter config provider" << std::endl; diff --git a/source/extensions/filters/http/composite/action.h b/source/extensions/filters/http/composite/action.h index e6507390ecb0..fffcd84e0d11 100644 --- a/source/extensions/filters/http/composite/action.h +++ b/source/extensions/filters/http/composite/action.h @@ -27,9 +27,6 @@ class ExecuteFilterActionFactory : public Logger::Loggable, public Matcher::ActionFactory { public: - ~ExecuteFilterActionFactory() { - std::cout << "execute action factory destructo called..." << std::endl; - } std::string name() const override { return "composite-action"; } Matcher::ActionFactoryCb @@ -45,7 +42,7 @@ class ExecuteFilterActionFactory std::unique_ptr< Envoy::Filter::DynamicFilterConfigProvider> provider_; - std::shared_ptr filter_config_provider_manager_; +// std::shared_ptr filter_config_provider_manager_; }; DECLARE_FACTORY(ExecuteFilterActionFactory); From 44be019a46a212261a781efec550cc38d430cda6 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 30 Aug 2023 18:46:52 +0530 Subject: [PATCH 12/73] format Signed-off-by: Rama Chavali --- source/extensions/filters/http/composite/action.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/source/extensions/filters/http/composite/action.h b/source/extensions/filters/http/composite/action.h index fffcd84e0d11..543ae0207923 100644 --- a/source/extensions/filters/http/composite/action.h +++ b/source/extensions/filters/http/composite/action.h @@ -39,10 +39,8 @@ class ExecuteFilterActionFactory } private: - std::unique_ptr< - Envoy::Filter::DynamicFilterConfigProvider> + std::unique_ptr> provider_; -// std::shared_ptr filter_config_provider_manager_; }; DECLARE_FACTORY(ExecuteFilterActionFactory); From eba6a4631f93ea72805f8e54a53f8be52223d820 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 30 Aug 2023 18:58:46 +0530 Subject: [PATCH 13/73] format again Signed-off-by: Rama Chavali --- source/extensions/filters/http/composite/action.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/http/composite/action.h b/source/extensions/filters/http/composite/action.h index 543ae0207923..6d2319b7ea19 100644 --- a/source/extensions/filters/http/composite/action.h +++ b/source/extensions/filters/http/composite/action.h @@ -39,7 +39,8 @@ class ExecuteFilterActionFactory } private: - std::unique_ptr> + std::unique_ptr< + Envoy::Filter::DynamicFilterConfigProvider> provider_; }; From 822a20070673d7fa8124b8aba1746eb51674aa98 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 30 Aug 2023 21:29:17 +0530 Subject: [PATCH 14/73] move provider manager as class member Signed-off-by: Rama Chavali --- source/extensions/filters/http/composite/action.cc | 6 +++--- source/extensions/filters/http/composite/action.h | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 0c81ead3d829..f2a16d8ee6c6 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -21,14 +21,14 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( context.server_factory_context_.value(); std::cout << "creating singleton manager and provider" << std::endl; Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); - std::shared_ptr filter_config_provider_manager = + filter_config_provider_manager_ = Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( server_factory_context); std::cout << "created filter config provider manager" << std::endl; - if (filter_config_provider_manager == nullptr) { + if (filter_config_provider_manager_ == nullptr) { throw EnvoyException("Failed to create filter config provider manager"); } - provider_ = filter_config_provider_manager->createDynamicFilterConfigProvider( + provider_ = filter_config_provider_manager_->createDynamicFilterConfigProvider( config_discovery, composite_action.dynamic_config().name(), server_factory_context, factory_context, server_factory_context.clusterManager(), false, "http", nullptr); std::cout << "created dynamic filter config provider" << std::endl; diff --git a/source/extensions/filters/http/composite/action.h b/source/extensions/filters/http/composite/action.h index 6d2319b7ea19..d1c07acfcab0 100644 --- a/source/extensions/filters/http/composite/action.h +++ b/source/extensions/filters/http/composite/action.h @@ -39,6 +39,7 @@ class ExecuteFilterActionFactory } private: + std::shared_ptr filter_config_provider_manager_; std::unique_ptr< Envoy::Filter::DynamicFilterConfigProvider> provider_; From 96851bcc7457a8c5201b5b1ccea2a348409a4622 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 31 Aug 2023 09:36:49 +0530 Subject: [PATCH 15/73] enable full test Signed-off-by: Rama Chavali --- .../composite_filter_integration_test.cc | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index 38675fafc959..4764873c2b56 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -178,7 +178,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, CompositeFilterIntegrationTest, // Verifies that if we don't match the match action the request is proxied as normal, while if the // match action is hit we apply the specified filter to the stream. TEST_P(CompositeFilterIntegrationTest, TestBasic) { - prependCompositeDynamicFilter(); + prependCompositeFilter(); initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -200,26 +200,26 @@ TEST_P(CompositeFilterIntegrationTest, TestBasic) { // Verifies that if we don't match the match action the request is proxied as normal, while if the // match action is hit we apply the specified dynamic filter to the stream. -// TEST_P(CompositeFilterIntegrationTest, TestBasicDynamicFilter) { -// prependCompositeDynamicFilter("composite-dynamic"); -// initialize(); -// codec_client_ = makeHttpConnection(lookupPort("http")); - -// { -// auto response = codec_client_->makeRequestWithBody(default_request_headers_, 1024); -// waitForNextUpstreamRequest(); - -// upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); -// ASSERT_TRUE(response->waitForEndStream()); -// EXPECT_THAT(response->headers(), Http::HttpStatusIs("200")); -// } - -// { -// auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024); -// ASSERT_TRUE(response->waitForEndStream()); -// EXPECT_THAT(response->headers(), Http::HttpStatusIs("403")); -// } -// } +TEST_P(CompositeFilterIntegrationTest, TestBasicDynamicFilter) { + prependCompositeDynamicFilter("composite-dynamic"); + initialize(); + codec_client_ = makeHttpConnection(lookupPort("http")); + + { + auto response = codec_client_->makeRequestWithBody(default_request_headers_, 1024); + waitForNextUpstreamRequest(); + + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_THAT(response->headers(), Http::HttpStatusIs("200")); + } + + { + auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_THAT(response->headers(), Http::HttpStatusIs("403")); + } +} // Verifies function of the per-route config in the ExtensionWithMatcher class. TEST_P(CompositeFilterIntegrationTest, TestPerRoute) { From 6f8659d12e9a9843df57c2b8102bb07114a6e368 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 31 Aug 2023 14:22:59 +0530 Subject: [PATCH 16/73] more debug Signed-off-by: Rama Chavali --- source/common/filter/config_discovery_impl.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index 44afdb315926..90b668cdd1f7 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -89,6 +89,16 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa // Extend the lifetime of TLS by capturing main_config_, because // otherwise, the callback to clear TLS worker content is not executed. [main_config = main_config_]() { + if (main_config == nullptr) { + std::cout << "main config is nuil" + << "\n"; + return; + } + if (main_config->tls_ == nullptr) { + std::cout << "main_config->tls_ is nuil" + << "\n"; + return; + } // Explicitly delete TLS on the main thread. main_config->tls_.reset(); // Explicitly clear the last config instance here in case it has its From d67f42682385b009e27a79ccb20f3cf8cc57eff1 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 31 Aug 2023 16:34:54 +0530 Subject: [PATCH 17/73] remove test case Signed-off-by: Rama Chavali --- source/common/filter/config_discovery_impl.h | 10 ---------- .../composite_filter_integration_test.cc | 17 +++-------------- 2 files changed, 3 insertions(+), 24 deletions(-) diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index 90b668cdd1f7..44afdb315926 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -89,16 +89,6 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa // Extend the lifetime of TLS by capturing main_config_, because // otherwise, the callback to clear TLS worker content is not executed. [main_config = main_config_]() { - if (main_config == nullptr) { - std::cout << "main config is nuil" - << "\n"; - return; - } - if (main_config->tls_ == nullptr) { - std::cout << "main_config->tls_ is nuil" - << "\n"; - return; - } // Explicitly delete TLS on the main thread. main_config->tls_.reset(); // Explicitly clear the last config instance here in case it has its diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index 4764873c2b56..af1220cf9c51 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -205,20 +205,9 @@ TEST_P(CompositeFilterIntegrationTest, TestBasicDynamicFilter) { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); - { - auto response = codec_client_->makeRequestWithBody(default_request_headers_, 1024); - waitForNextUpstreamRequest(); - - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_THAT(response->headers(), Http::HttpStatusIs("200")); - } - - { - auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024); - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_THAT(response->headers(), Http::HttpStatusIs("403")); - } + auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_THAT(response->headers(), Http::HttpStatusIs("403")); } // Verifies function of the per-route config in the ExtensionWithMatcher class. From b9085fe29717be6370b6d0e2e857986d9db8ea71 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 31 Aug 2023 16:59:30 +0530 Subject: [PATCH 18/73] add test case back and log Signed-off-by: Rama Chavali --- source/common/filter/config_discovery_impl.h | 1 + .../composite_filter_integration_test.cc | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index 44afdb315926..0f76c0c31fc0 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -83,6 +83,7 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa default_configuration_(std::move(default_config)){}; ~DynamicFilterConfigProviderImpl() override { + std::cout<<"Destroying DynamicFilterConfigProviderImpl"<tls_; if (!tls->isShutdown()) { tls->runOnAllThreads([](OptRef tls) { tls->config_ = {}; }, diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index af1220cf9c51..4764873c2b56 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -205,9 +205,20 @@ TEST_P(CompositeFilterIntegrationTest, TestBasicDynamicFilter) { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); - auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024); - ASSERT_TRUE(response->waitForEndStream()); - EXPECT_THAT(response->headers(), Http::HttpStatusIs("403")); + { + auto response = codec_client_->makeRequestWithBody(default_request_headers_, 1024); + waitForNextUpstreamRequest(); + + upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_THAT(response->headers(), Http::HttpStatusIs("200")); + } + + { + auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_THAT(response->headers(), Http::HttpStatusIs("403")); + } } // Verifies function of the per-route config in the ExtensionWithMatcher class. From 6317691c715cf6dd32b92a85356c49468a17fea4 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 31 Aug 2023 18:56:15 +0530 Subject: [PATCH 19/73] fix format Signed-off-by: Rama Chavali --- source/common/filter/config_discovery_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index 0f76c0c31fc0..fb9f2ad43439 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -83,7 +83,7 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa default_configuration_(std::move(default_config)){}; ~DynamicFilterConfigProviderImpl() override { - std::cout<<"Destroying DynamicFilterConfigProviderImpl"<tls_; if (!tls->isShutdown()) { tls->runOnAllThreads([](OptRef tls) { tls->config_ = {}; }, From 8db6396e90aa99f50939be098ad19d618ac6a7c3 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Fri, 1 Sep 2023 16:52:42 +0530 Subject: [PATCH 20/73] more debug logs Signed-off-by: Rama Chavali --- source/common/filter/config_discovery_impl.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index fb9f2ad43439..37bc3a451cd4 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -544,6 +544,7 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa auto subscription = getSubscription(config_source.config_source(), filter_config_name, server_context, cluster_manager, subscription_stat_prefix); + std::cout << "obtained subscription" << std::endl; // For warming, wait until the subscription receives the first response to indicate readiness. // Otherwise, mark ready immediately and start the subscription on initialization. A default // config is expected in the latter case. @@ -562,7 +563,7 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa getDefaultConfig(config_source.default_config(), filter_config_name, server_context, last_filter_in_filter_chain, filter_chain_type, require_type_urls); } - + std::cout << "creating dynammic filter config provider impl" << std::endl; std::unique_ptr> provider = std::make_unique(subscription, require_type_urls, server_context, factory_context, std::move(default_config), @@ -573,7 +574,9 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa if (config_source.apply_default_config_without_warming()) { factory_context.initManager().add(provider->initTarget()); } + std::cout << "applying last or default config" << std::endl; applyLastOrDefaultConfig(subscription, *provider, filter_config_name); + std::cout << "applied last or default config" << std::endl; return provider; } From b23d5f3b972251a36810b7b64c0d4d28a290d9af Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sat, 2 Sep 2023 12:10:14 +0530 Subject: [PATCH 21/73] comment dest Signed-off-by: Rama Chavali --- source/common/filter/config_discovery_impl.h | 29 ++++++++++--------- .../filters/http/composite/action.cc | 1 + .../filters/http/composite/action.h | 4 +++ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index 37bc3a451cd4..eb916bb79e26 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -84,19 +84,19 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa ~DynamicFilterConfigProviderImpl() override { std::cout << "Destroying DynamicFilterConfigProviderImpl" << std::endl; - auto& tls = main_config_->tls_; - if (!tls->isShutdown()) { - tls->runOnAllThreads([](OptRef tls) { tls->config_ = {}; }, - // Extend the lifetime of TLS by capturing main_config_, because - // otherwise, the callback to clear TLS worker content is not executed. - [main_config = main_config_]() { - // Explicitly delete TLS on the main thread. - main_config->tls_.reset(); - // Explicitly clear the last config instance here in case it has its - // own TLS. - main_config->current_config_ = {}; - }); - } + // auto& tls = main_config_->tls_; + // if (!tls->isShutdown()) { + // tls->runOnAllThreads([](OptRef tls) { tls->config_ = {}; }, + // // Extend the lifetime of TLS by capturing main_config_, because + // // otherwise, the callback to clear TLS worker content is not + // executed. [main_config = main_config_]() { + // // Explicitly delete TLS on the main thread. + // main_config->tls_.reset(); + // // Explicitly clear the last config instance here in case it has its + // // own TLS. + // main_config->current_config_ = {}; + // }); + // } } // Config::ExtensionConfigProvider @@ -634,6 +634,9 @@ class HttpFilterConfigProviderManagerImpl Server::Configuration::NamedHttpFilterConfigFactory>> { public: absl::string_view statPrefix() const override { return "http_filter."; } + ~HttpFilterConfigProviderManagerImpl() { + std::cout << "Destroying HttpFilterConfigProviderManagerImpl" << std::endl; + } protected: bool diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index f2a16d8ee6c6..9e9a9ffe2d33 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -28,6 +28,7 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( if (filter_config_provider_manager_ == nullptr) { throw EnvoyException("Failed to create filter config provider manager"); } + provider_ = filter_config_provider_manager_->createDynamicFilterConfigProvider( config_discovery, composite_action.dynamic_config().name(), server_factory_context, factory_context, server_factory_context.clusterManager(), false, "http", nullptr); diff --git a/source/extensions/filters/http/composite/action.h b/source/extensions/filters/http/composite/action.h index d1c07acfcab0..4022d3388ee1 100644 --- a/source/extensions/filters/http/composite/action.h +++ b/source/extensions/filters/http/composite/action.h @@ -16,6 +16,7 @@ class ExecuteFilterAction envoy::extensions::filters::http::composite::v3::ExecuteFilterAction> { public: explicit ExecuteFilterAction(Http::FilterFactoryCb cb) : cb_(std::move(cb)) {} + ~ExecuteFilterAction() { std::cout << "ExecuteFilterAction destructor" << std::endl; } void createFilters(Http::FilterChainFactoryCallbacks& callbacks) const; @@ -27,6 +28,9 @@ class ExecuteFilterActionFactory : public Logger::Loggable, public Matcher::ActionFactory { public: + ~ExecuteFilterActionFactory() { + std::cout << "ExecuteFilterActionFactory destructor" << std::endl; + } std::string name() const override { return "composite-action"; } Matcher::ActionFactoryCb From f1abfc49818fe91707e0d4c5f244094e118dc726 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 5 Sep 2023 17:14:40 +0530 Subject: [PATCH 22/73] hold providers in provider manager Signed-off-by: Rama Chavali --- envoy/filter/config_provider_manager.h | 22 ++++++ source/common/filter/config_discovery_impl.h | 76 +++++++++++++------ .../filters/http/composite/action.cc | 16 ++-- .../filters/http/composite/action.h | 10 --- 4 files changed, 79 insertions(+), 45 deletions(-) diff --git a/envoy/filter/config_provider_manager.h b/envoy/filter/config_provider_manager.h index 39fe6273e5a8..e34bbcaaa558 100644 --- a/envoy/filter/config_provider_manager.h +++ b/envoy/filter/config_provider_manager.h @@ -68,6 +68,28 @@ template class FilterConfigProviderManager { * Get the stat prefix for the scope of the filter provider manager. */ virtual absl::string_view statPrefix() const PURE; + + /** + * Get an FilterConfigProviderPtr for a filter config. The config providers may share + * the underlying subscriptions to the filter config discovery service. + * @param config_source supplies the extension configuration source for the filter configs. + * @param filter_config_name the filter config resource name. + * @param factory_context is the context to use for the filter config provider. + * @param last_filter_in_filter_chain indicates whether this filter is the last filter in the + * configured chain + * @param filter_chain_type is the filter chain type + * @param listener_filter_matcher is the filter matcher for TCP listener filter. nullptr for other + * filter types. + */ + virtual void registerDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, FactoryCtx& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) PURE; + + virtual OptRef dynamicProviderConfig(const std::string& filter_config_name) PURE; }; } // namespace Filter diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index eb916bb79e26..d8e30f57cdd0 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -83,20 +83,20 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa default_configuration_(std::move(default_config)){}; ~DynamicFilterConfigProviderImpl() override { - std::cout << "Destroying DynamicFilterConfigProviderImpl" << std::endl; - // auto& tls = main_config_->tls_; - // if (!tls->isShutdown()) { - // tls->runOnAllThreads([](OptRef tls) { tls->config_ = {}; }, - // // Extend the lifetime of TLS by capturing main_config_, because - // // otherwise, the callback to clear TLS worker content is not - // executed. [main_config = main_config_]() { - // // Explicitly delete TLS on the main thread. - // main_config->tls_.reset(); - // // Explicitly clear the last config instance here in case it has its - // // own TLS. - // main_config->current_config_ = {}; - // }); - // } + auto& tls = main_config_->tls_; + if (!tls->isShutdown()) { + tls->runOnAllThreads([](OptRef tls) { tls->config_ = {}; }, + // Extend the lifetime of TLS by capturing main_config_, because + // otherwise, the callback to clear TLS worker content is not + // executed. + [main_config = main_config_]() { + // Explicitly delete TLS on the main thread. + main_config->tls_.reset(); + // Explicitly clear the last config instance here in case it has its + // own TLS. + main_config->current_config_ = {}; + }); + } } // Config::ExtensionConfigProvider @@ -536,15 +536,38 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override { + return createDynamicFilterConfigProviderInternal( + config_source, filter_config_name, server_context, factory_context, cluster_manager, + last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); + } + + void registerDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, FactoryCtx& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override { + auto provider = createDynamicFilterConfigProviderInternal( + config_source, filter_config_name, server_context, factory_context, cluster_manager, + last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); + dynamic_providers_[filter_config_name] = std::move(provider); + } + + std::unique_ptr> + createDynamicFilterConfigProviderInternal( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, FactoryCtx& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) { std::string subscription_stat_prefix; absl::string_view provider_stat_prefix; - subscription_stat_prefix = - absl::StrCat("extension_config_discovery.", statPrefix(), filter_config_name, "."); - provider_stat_prefix = subscription_stat_prefix; + subscription_stat_prefix = provider_stat_prefix = subscription_stat_prefix; auto subscription = getSubscription(config_source.config_source(), filter_config_name, server_context, cluster_manager, subscription_stat_prefix); - std::cout << "obtained subscription" << std::endl; // For warming, wait until the subscription receives the first response to indicate readiness. // Otherwise, mark ready immediately and start the subscription on initialization. A default // config is expected in the latter case. @@ -563,7 +586,6 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa getDefaultConfig(config_source.default_config(), filter_config_name, server_context, last_filter_in_filter_chain, filter_chain_type, require_type_urls); } - std::cout << "creating dynammic filter config provider impl" << std::endl; std::unique_ptr> provider = std::make_unique(subscription, require_type_urls, server_context, factory_context, std::move(default_config), @@ -574,9 +596,7 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa if (config_source.apply_default_config_without_warming()) { factory_context.initManager().add(provider->initTarget()); } - std::cout << "applying last or default config" << std::endl; applyLastOrDefaultConfig(subscription, *provider, filter_config_name); - std::cout << "applied last or default config" << std::endl; return provider; } @@ -586,6 +606,14 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa return std::make_unique>(config, filter_config_name); } + OptRef dynamicProviderConfig(const std::string& filter_config_name) override { + auto provider = dynamic_providers_.find(filter_config_name); + if (provider == dynamic_providers_.end()) { + return absl::nullopt; + } + return provider->second->config(); + } + absl::string_view statPrefix() const override PURE; std::tuple @@ -599,6 +627,9 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa } protected: + absl::flat_hash_map>> + dynamic_providers_; + virtual void validateFilters(const std::string&, const std::string&, const std::string&, bool, bool) const {}; virtual bool isTerminalFilter(Factory*, Protobuf::Message&, @@ -634,9 +665,6 @@ class HttpFilterConfigProviderManagerImpl Server::Configuration::NamedHttpFilterConfigFactory>> { public: absl::string_view statPrefix() const override { return "http_filter."; } - ~HttpFilterConfigProviderManagerImpl() { - std::cout << "Destroying HttpFilterConfigProviderManagerImpl" << std::endl; - } protected: bool diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 9e9a9ffe2d33..c76b0f26363e 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -19,22 +19,16 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( auto config_discovery = composite_action.dynamic_config().config_discovery(); Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); - std::cout << "creating singleton manager and provider" << std::endl; Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); - filter_config_provider_manager_ = + std::shared_ptr filter_config_provider_manager = Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( server_factory_context); - std::cout << "created filter config provider manager" << std::endl; - if (filter_config_provider_manager_ == nullptr) { - throw EnvoyException("Failed to create filter config provider manager"); - } - - provider_ = filter_config_provider_manager_->createDynamicFilterConfigProvider( + filter_config_provider_manager->registerDynamicFilterConfigProvider( config_discovery, composite_action.dynamic_config().name(), server_factory_context, factory_context, server_factory_context.clusterManager(), false, "http", nullptr); - std::cout << "created dynamic filter config provider" << std::endl; - return [this]() -> Matcher::ActionPtr { - auto config_value = provider_->config(); + return [name = composite_action.dynamic_config().name(), + filter_config_provider_manager]() -> Matcher::ActionPtr { + auto config_value = filter_config_provider_manager->dynamicProviderConfig(name); if (!config_value.has_value()) { throw EnvoyException("Failed to get dynamic config for filter"); } diff --git a/source/extensions/filters/http/composite/action.h b/source/extensions/filters/http/composite/action.h index 4022d3388ee1..b78553d8ab77 100644 --- a/source/extensions/filters/http/composite/action.h +++ b/source/extensions/filters/http/composite/action.h @@ -16,7 +16,6 @@ class ExecuteFilterAction envoy::extensions::filters::http::composite::v3::ExecuteFilterAction> { public: explicit ExecuteFilterAction(Http::FilterFactoryCb cb) : cb_(std::move(cb)) {} - ~ExecuteFilterAction() { std::cout << "ExecuteFilterAction destructor" << std::endl; } void createFilters(Http::FilterChainFactoryCallbacks& callbacks) const; @@ -28,9 +27,6 @@ class ExecuteFilterActionFactory : public Logger::Loggable, public Matcher::ActionFactory { public: - ~ExecuteFilterActionFactory() { - std::cout << "ExecuteFilterActionFactory destructor" << std::endl; - } std::string name() const override { return "composite-action"; } Matcher::ActionFactoryCb @@ -41,12 +37,6 @@ class ExecuteFilterActionFactory ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique(); } - -private: - std::shared_ptr filter_config_provider_manager_; - std::unique_ptr< - Envoy::Filter::DynamicFilterConfigProvider> - provider_; }; DECLARE_FACTORY(ExecuteFilterActionFactory); From 72d822e65934cf8a83367236f02a7a2b888cd23d Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Mon, 11 Sep 2023 17:08:04 +0530 Subject: [PATCH 23/73] move providers to FilterChainFactoryContext Signed-off-by: Rama Chavali --- envoy/filter/config_provider_manager.h | 22 ----------- envoy/server/factory_context.h | 30 +++++++++++++++ source/common/filter/config_discovery_impl.h | 37 ------------------- .../filters/http/composite/action.cc | 17 ++++----- .../listener_managers/listener_manager/BUILD | 2 + .../filter_chain_manager_impl.cc | 28 ++++++++++++++ .../filter_chain_manager_impl.h | 22 +++++++++++ .../listener_manager/listener_impl.cc | 27 ++++++++++++++ .../listener_manager/listener_impl.h | 20 ++++++++++ source/server/factory_context_impl.cc | 8 ++++ source/server/factory_context_impl.h | 10 +++++ test/mocks/server/factory_context.h | 13 +++++++ 12 files changed, 167 insertions(+), 69 deletions(-) diff --git a/envoy/filter/config_provider_manager.h b/envoy/filter/config_provider_manager.h index e34bbcaaa558..39fe6273e5a8 100644 --- a/envoy/filter/config_provider_manager.h +++ b/envoy/filter/config_provider_manager.h @@ -68,28 +68,6 @@ template class FilterConfigProviderManager { * Get the stat prefix for the scope of the filter provider manager. */ virtual absl::string_view statPrefix() const PURE; - - /** - * Get an FilterConfigProviderPtr for a filter config. The config providers may share - * the underlying subscriptions to the filter config discovery service. - * @param config_source supplies the extension configuration source for the filter configs. - * @param filter_config_name the filter config resource name. - * @param factory_context is the context to use for the filter config provider. - * @param last_filter_in_filter_chain indicates whether this filter is the last filter in the - * configured chain - * @param filter_chain_type is the filter chain type - * @param listener_filter_matcher is the filter matcher for TCP listener filter. nullptr for other - * filter types. - */ - virtual void registerDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, FactoryCtx& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) PURE; - - virtual OptRef dynamicProviderConfig(const std::string& filter_config_name) PURE; }; } // namespace Filter diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index e665289120d1..7ca3a19e2d2d 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -272,6 +272,36 @@ class FactoryContext : public virtual ListenerAccessLogFactoryContext { * @return Router::Context& a reference to the router context. */ virtual Router::Context& routerContext() PURE; + + /** + * Create an FilterConfigProviderPtr for a filter config. The config providers may share + * the underlying subscriptions to the filter config discovery service. + * @param config_source supplies the extension configuration source for the filter configs. + * @param filter_config_name the filter config resource name. + * @param factory_context is the context to use for the filter config provider. + * @param last_filter_in_filter_chain indicates whether this filter is the last filter in the + * configured chain + * @param filter_chain_type is the filter chain type + * @param listener_filter_matcher is the filter matcher for TCP listener filter. nullptr for other + * filter types. + */ + virtual void createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, + Server::Configuration::FactoryContext& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) PURE; + + /** + * Get configuration for dynamic provider. + * @param filter_config_name the filter config resource name. + * + * @return OptRef the filter factory callback. + */ + virtual OptRef + dynamicProviderConfig(const std::string& filter_config_name) PURE; }; /** diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index 99c4b6690373..91dcc33bbffc 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -540,32 +540,6 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override { - return createDynamicFilterConfigProviderInternal( - config_source, filter_config_name, server_context, factory_context, cluster_manager, - last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); - } - - void registerDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, FactoryCtx& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override { - auto provider = createDynamicFilterConfigProviderInternal( - config_source, filter_config_name, server_context, factory_context, cluster_manager, - last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); - dynamic_providers_[filter_config_name] = std::move(provider); - } - - std::unique_ptr> - createDynamicFilterConfigProviderInternal( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, FactoryCtx& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) { std::string subscription_stat_prefix; absl::string_view provider_stat_prefix; subscription_stat_prefix = provider_stat_prefix = subscription_stat_prefix; @@ -610,14 +584,6 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa return std::make_unique>(config, filter_config_name); } - OptRef dynamicProviderConfig(const std::string& filter_config_name) override { - auto provider = dynamic_providers_.find(filter_config_name); - if (provider == dynamic_providers_.end()) { - return absl::nullopt; - } - return provider->second->config(); - } - absl::string_view statPrefix() const override PURE; std::tuple @@ -631,9 +597,6 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa } protected: - absl::flat_hash_map>> - dynamic_providers_; - virtual void validateFilters(const std::string&, const std::string&, const std::string&, bool, bool) const {}; virtual bool isTerminalFilter(Factory*, Protobuf::Message&, diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index c76b0f26363e..7e803b98dad8 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -20,20 +20,17 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); - std::shared_ptr filter_config_provider_manager = - Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( - server_factory_context); - filter_config_provider_manager->registerDynamicFilterConfigProvider( + // Create a dynamic filter config provider and register it with the server factory context. + factory_context.createDynamicFilterConfigProvider( config_discovery, composite_action.dynamic_config().name(), server_factory_context, factory_context, server_factory_context.clusterManager(), false, "http", nullptr); - return [name = composite_action.dynamic_config().name(), - filter_config_provider_manager]() -> Matcher::ActionPtr { - auto config_value = filter_config_provider_manager->dynamicProviderConfig(name); - if (!config_value.has_value()) { + return [&factory_context = factory_context, + name = composite_action.dynamic_config().name()]() -> Matcher::ActionPtr { + auto factory_cb = factory_context.dynamicProviderConfig(name); + if (!factory_cb.has_value()) { throw EnvoyException("Failed to get dynamic config for filter"); } - auto factory_cb = config_value.value().get().factory_cb; - return std::make_unique(std::move(factory_cb)); + return std::make_unique(factory_cb.value()); }; } else { // Static filter configuration. diff --git a/source/extensions/listener_managers/listener_manager/BUILD b/source/extensions/listener_managers/listener_manager/BUILD index ddb413ac5319..1387d958ef25 100644 --- a/source/extensions/listener_managers/listener_manager/BUILD +++ b/source/extensions/listener_managers/listener_manager/BUILD @@ -111,6 +111,8 @@ envoy_cc_library( "//envoy/server:transport_socket_config_interface", "//source/common/common:empty_string", "//source/common/config:utility_lib", + "//source/common/filter:config_discovery_lib", + "//source/common/http:filter_chain_helper_lib", "//source/common/init:manager_lib", "//source/common/matcher:matcher_lib", "//source/common/network:cidr_range_lib", diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc index 356072181bb0..ec0ce526ee98 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc @@ -6,6 +6,7 @@ #include "source/common/common/empty_string.h" #include "source/common/common/fmt.h" #include "source/common/config/utility.h" +#include "source/common/http/filter_chain_helper.h" #include "source/common/matcher/matcher.h" #include "source/common/network/matching/data_impl.h" #include "source/common/network/matching/inputs.h" @@ -193,6 +194,33 @@ bool PerFilterChainFactoryContextImpl::isQuicListener() const { return parent_context_.isQuicListener(); } +void PerFilterChainFactoryContextImpl::createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_factory_context, + Server::Configuration::FactoryContext& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) { + if (filter_config_provider_manager_ == nullptr) { + filter_config_provider_manager_ = + std::make_shared(); + } + auto provider = filter_config_provider_manager_->createDynamicFilterConfigProvider( + config_source, filter_config_name, server_factory_context, factory_context, cluster_manager, + last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); + dynamic_providers_[filter_config_name] = std::move(provider); +} + +OptRef +PerFilterChainFactoryContextImpl::dynamicProviderConfig(const std::string& filter_config_name) { + auto provider = dynamic_providers_.find(filter_config_name); + if (provider == dynamic_providers_.end()) { + return absl::nullopt; + } + return provider->second->config()->factory_cb; +} + FilterChainManagerImpl::FilterChainManagerImpl( const std::vector& addresses, Configuration::FactoryContext& factory_context, Init::Manager& init_manager, diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h index 7addd4f710ff..6b0478dd6834 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h @@ -17,6 +17,7 @@ #include "envoy/thread_local/thread_local.h" #include "source/common/common/logger.h" +#include "source/common/filter/config_discovery_impl.h" #include "source/common/init/manager_impl.h" #include "source/common/network/cidr_range.h" #include "source/common/network/lc_trie.h" @@ -50,6 +51,9 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor explicit PerFilterChainFactoryContextImpl(Configuration::FactoryContext& parent_context, Init::Manager& init_manager); + ~PerFilterChainFactoryContextImpl() { + std::cout << "Destructing PerFilterChainFactoryContextImpl >>>>>>>>>>>>>>>>>>>>" << std::endl; + } // DrainDecision bool drainClose() const override; Common::CallbackHandlePtr addOnDrainCloseCb(DrainCloseCb) const override { @@ -89,6 +93,16 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; + void createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, + Server::Configuration::FactoryContext& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; + OptRef + dynamicProviderConfig(const std::string& filter_config_name) override; void startDraining() override { is_draining_.store(true); } @@ -100,6 +114,14 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor Stats::ScopeSharedPtr filter_chain_scope_; Init::Manager& init_manager_; std::atomic is_draining_{false}; + // TODO(ramaraochavali): Move DownstreamFilterConfigProviderManager usages to use this instead of + // current singleton. + std::shared_ptr> + filter_config_provider_manager_{}; + absl::flat_hash_map>> + dynamic_providers_; }; using FilterChainActionFactoryContext = Configuration::ServerFactoryContext; diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.cc b/source/extensions/listener_managers/listener_manager/listener_impl.cc index 832cb69bfaf4..610f1efccf98 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_impl.cc @@ -299,6 +299,15 @@ ListenerFactoryContextBaseImpl::getTransportSocketFactoryContext() const { } Stats::Scope& ListenerFactoryContextBaseImpl::listenerScope() { return *listener_scope_; } bool ListenerFactoryContextBaseImpl::isQuicListener() const { return is_quic_; } +void ListenerFactoryContextBaseImpl::createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, + Server::Configuration::ServerFactoryContext&, Server::Configuration::FactoryContext&, + Upstream::ClusterManager&, bool, const std::string&, + const Network::ListenerFilterMatcherSharedPtr&) {} +OptRef +ListenerFactoryContextBaseImpl::dynamicProviderConfig(const std::string&) { + return absl::nullopt; +} Network::DrainDecision& ListenerFactoryContextBaseImpl::drainDecision() { return *this; } Server::DrainManager& ListenerFactoryContextBaseImpl::drainManager() { return *drain_manager_; } Init::Manager& ListenerFactoryContextBaseImpl::initManager() { return server_.initManager(); } @@ -911,10 +920,12 @@ const Envoy::Config::TypedMetadata& PerListenerFactoryContextImpl::listenerTyped envoy::config::core::v3::TrafficDirection PerListenerFactoryContextImpl::direction() const { return listener_factory_context_base_->direction(); }; + TimeSource& PerListenerFactoryContextImpl::timeSource() { return api().timeSource(); } const Network::ListenerConfig& PerListenerFactoryContextImpl::listenerConfig() const { return *listener_config_; } + ProtobufMessage::ValidationContext& PerListenerFactoryContextImpl::messageValidationContext() { return getServerFactoryContext().messageValidationContext(); } @@ -942,6 +953,22 @@ Stats::Scope& PerListenerFactoryContextImpl::listenerScope() { bool PerListenerFactoryContextImpl::isQuicListener() const { return listener_factory_context_base_->isQuicListener(); } +void PerListenerFactoryContextImpl::createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, + Server::Configuration::FactoryContext& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) { + return listener_factory_context_base_->createDynamicFilterConfigProvider( + config_source, filter_config_name, server_context, factory_context, cluster_manager, + last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); +} +OptRef +PerListenerFactoryContextImpl::dynamicProviderConfig(const std::string& filter_config_name) { + return listener_factory_context_base_->dynamicProviderConfig(filter_config_name); +} Init::Manager& PerListenerFactoryContextImpl::initManager() { return listener_impl_.initManager(); } bool ListenerImpl::createNetworkFilterChain( diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.h b/source/extensions/listener_managers/listener_manager/listener_impl.h index 98a139e3a0f0..ca777bf914ca 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_impl.h @@ -163,6 +163,16 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; + void createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, + Server::Configuration::FactoryContext& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; + OptRef + dynamicProviderConfig(const std::string& filter_config_name) override; // DrainDecision bool drainClose() const override { @@ -241,6 +251,16 @@ class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryConte Stats::Scope& listenerScope() override; bool isQuicListener() const override; + void createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, + Server::Configuration::FactoryContext& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; + OptRef + dynamicProviderConfig(const std::string& filter_config_name) override; // ListenerFactoryContext const Network::ListenerConfig& listenerConfig() const override; diff --git a/source/server/factory_context_impl.cc b/source/server/factory_context_impl.cc index 3d525653814c..8c86ac4397bc 100644 --- a/source/server/factory_context_impl.cc +++ b/source/server/factory_context_impl.cc @@ -61,6 +61,14 @@ envoy::config::core::v3::TrafficDirection FactoryContextImpl::direction() const Network::DrainDecision& FactoryContextImpl::drainDecision() { return drain_decision_; } Stats::Scope& FactoryContextImpl::listenerScope() { return listener_scope_; } bool FactoryContextImpl::isQuicListener() const { return is_quic_; } +void FactoryContextImpl::createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, + Server::Configuration::ServerFactoryContext&, Server::Configuration::FactoryContext&, + Upstream::ClusterManager&, bool, const std::string&, + const Network::ListenerFilterMatcherSharedPtr&) {} +OptRef FactoryContextImpl::dynamicProviderConfig(const std::string&) { + return absl::nullopt; +} } // namespace Server } // namespace Envoy diff --git a/source/server/factory_context_impl.h b/source/server/factory_context_impl.h index 5fdfd022e5b0..7f7ecd7631b2 100644 --- a/source/server/factory_context_impl.h +++ b/source/server/factory_context_impl.h @@ -47,6 +47,16 @@ class FactoryContextImpl : public Configuration::FactoryContext { Network::DrainDecision& drainDecision() override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; + void createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, + Server::Configuration::FactoryContext& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; + OptRef + dynamicProviderConfig(const std::string& filter_config_name) override; private: Server::Instance& server_; diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index 756160cb1c0c..087502b5a819 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -57,6 +57,19 @@ class MockFactoryContext : public virtual ListenerFactoryContext { MOCK_METHOD(ProtobufMessage::ValidationVisitor&, messageValidationVisitor, ()); MOCK_METHOD(Api::Api&, api, ()); + void createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, + Server::Configuration::FactoryContext& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) {} + + OptRef dynamicProviderConfig(const std::string& filter_config_name) { + return absl::nullopt; + } + testing::NiceMock server_factory_context_; testing::NiceMock access_log_manager_; testing::NiceMock cluster_manager_; From 5b435447713f0e36fdf34721d40ab5eac6b3c9af Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Mon, 11 Sep 2023 17:46:44 +0530 Subject: [PATCH 24/73] format Signed-off-by: Rama Chavali --- test/mocks/server/factory_context.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index 087502b5a819..a7ac1e212485 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -64,9 +64,10 @@ class MockFactoryContext : public virtual ListenerFactoryContext { Server::Configuration::FactoryContext& factory_context, Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) {} + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override {} - OptRef dynamicProviderConfig(const std::string& filter_config_name) { + OptRef + dynamicProviderConfig(const std::string& filter_config_name) override { return absl::nullopt; } From ee23a672d057dcfa595e6b7f2046323b4bf1427f Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Mon, 11 Sep 2023 19:52:35 +0530 Subject: [PATCH 25/73] mock fixes Signed-off-by: Rama Chavali --- test/mocks/server/filter_chain_factory_context.h | 14 ++++++++++++++ test/mocks/server/listener_factory_context.h | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/test/mocks/server/filter_chain_factory_context.h b/test/mocks/server/filter_chain_factory_context.h index f09e7a565722..961504176436 100644 --- a/test/mocks/server/filter_chain_factory_context.h +++ b/test/mocks/server/filter_chain_factory_context.h @@ -11,6 +11,20 @@ class MockFilterChainFactoryContext : public MockFactoryContext, public FilterCh public: MockFilterChainFactoryContext(); ~MockFilterChainFactoryContext() override; + + void createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, + Server::Configuration::FactoryContext& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override {} + + OptRef + dynamicProviderConfig(const std::string& filter_config_name) override { + return absl::nullopt; + } }; } // namespace Configuration } // namespace Server diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index 5341b517d1ba..691fc0e8e581 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -56,6 +56,20 @@ class MockListenerFactoryContext : public ListenerFactoryContext { MOCK_METHOD(ProtobufMessage::ValidationVisitor&, messageValidationVisitor, ()); MOCK_METHOD(Api::Api&, api, ()); + void createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, + Server::Configuration::ServerFactoryContext& server_context, + Server::Configuration::FactoryContext& factory_context, + Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_chain_type, + const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override {} + + OptRef + dynamicProviderConfig(const std::string& filter_config_name) override { + return absl::nullopt; + } + testing::NiceMock server_factory_context_; testing::NiceMock access_log_manager_; testing::NiceMock cluster_manager_; From b9913f13a772fdb33efb499cc24b73b80c6055d1 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Mon, 11 Sep 2023 21:22:51 +0530 Subject: [PATCH 26/73] mock fixes Signed-off-by: Rama Chavali --- .../mocks/server/filter_chain_factory_context.h | 17 +++++++---------- test/mocks/server/listener_factory_context.h | 17 +++++++---------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/test/mocks/server/filter_chain_factory_context.h b/test/mocks/server/filter_chain_factory_context.h index 961504176436..451a69167f44 100644 --- a/test/mocks/server/filter_chain_factory_context.h +++ b/test/mocks/server/filter_chain_factory_context.h @@ -12,17 +12,14 @@ class MockFilterChainFactoryContext : public MockFactoryContext, public FilterCh MockFilterChainFactoryContext(); ~MockFilterChainFactoryContext() override; - void createDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, - Server::Configuration::FactoryContext& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override {} + void createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, + const std::string&, + Server::Configuration::ServerFactoryContext&, + Server::Configuration::FactoryContext&, + Upstream::ClusterManager&, bool, const std::string&, + const Network::ListenerFilterMatcherSharedPtr&) override {} - OptRef - dynamicProviderConfig(const std::string& filter_config_name) override { + OptRef dynamicProviderConfig(const std::string&) override { return absl::nullopt; } }; diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index 691fc0e8e581..83b55939d1f8 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -56,17 +56,14 @@ class MockListenerFactoryContext : public ListenerFactoryContext { MOCK_METHOD(ProtobufMessage::ValidationVisitor&, messageValidationVisitor, ()); MOCK_METHOD(Api::Api&, api, ()); - void createDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, - Server::Configuration::FactoryContext& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override {} + void createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, + const std::string&, + Server::Configuration::ServerFactoryContext&, + Server::Configuration::FactoryContext&, + Upstream::ClusterManager&, bool, const std::string&, + const Network::ListenerFilterMatcherSharedPtr&) override {} - OptRef - dynamicProviderConfig(const std::string& filter_config_name) override { + OptRef dynamicProviderConfig(const std::string&) override { return absl::nullopt; } From 9e5f060185b40ab888476f20923f483d08a4f9af Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 12 Sep 2023 10:07:45 +0530 Subject: [PATCH 27/73] mock fixes Signed-off-by: Rama Chavali --- test/mocks/server/factory_context.h | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index a7ac1e212485..f8a002dc0f41 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -57,17 +57,14 @@ class MockFactoryContext : public virtual ListenerFactoryContext { MOCK_METHOD(ProtobufMessage::ValidationVisitor&, messageValidationVisitor, ()); MOCK_METHOD(Api::Api&, api, ()); - void createDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, - Server::Configuration::FactoryContext& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override {} + void createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, + const std::string&, + Server::Configuration::ServerFactoryContext&, + Server::Configuration::FactoryContext&, + Upstream::ClusterManager&, bool, const std::string&, + const Network::ListenerFilterMatcherSharedPtr&) override {} - OptRef - dynamicProviderConfig(const std::string& filter_config_name) override { + OptRef dynamicProviderConfig(const std::string&) override { return absl::nullopt; } From d297afb4246e28c242d1715fb5faf63016951fdf Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 12 Sep 2023 14:58:41 +0530 Subject: [PATCH 28/73] stat fix Signed-off-by: Rama Chavali --- source/common/filter/config_discovery_impl.h | 4 +++- .../listener_manager/filter_chain_manager_impl.h | 3 --- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index 91dcc33bbffc..a90423025541 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -542,7 +542,9 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override { std::string subscription_stat_prefix; absl::string_view provider_stat_prefix; - subscription_stat_prefix = provider_stat_prefix = subscription_stat_prefix; + subscription_stat_prefix = + absl::StrCat("extension_config_discovery.", statPrefix(), filter_config_name, "."); + provider_stat_prefix = subscription_stat_prefix; auto subscription = getSubscription(config_source.config_source(), filter_config_name, server_context, cluster_manager, subscription_stat_prefix); diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h index 6b0478dd6834..09780bba10b0 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h @@ -51,9 +51,6 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor explicit PerFilterChainFactoryContextImpl(Configuration::FactoryContext& parent_context, Init::Manager& init_manager); - ~PerFilterChainFactoryContextImpl() { - std::cout << "Destructing PerFilterChainFactoryContextImpl >>>>>>>>>>>>>>>>>>>>" << std::endl; - } // DrainDecision bool drainClose() const override; Common::CallbackHandlePtr addOnDrainCloseCb(DrainCloseCb) const override { From dc407c44b91b5e9b700ad57a7e5050ece91ca454 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 12 Sep 2023 16:04:41 +0530 Subject: [PATCH 29/73] release notes Signed-off-by: Rama Chavali --- changelogs/current.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index c45710a1c4e0..aa13162b6c3a 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -240,11 +240,13 @@ new_features: change: | added :ref:`record_headers_received_time ` to control writing request and response headers received time in trace output. - - area: upstream change: | Added the ability to specify a custom upstream local address selector using :ref:`local_address_selector:`. +- area: composite filter + added :ref:`ExtensionConfiguration discovery service` support for + :ref:`composite filter `. deprecated: - area: tracing From 4d1cb8344febb107499b272c6f4bad41e68b9704 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 12 Sep 2023 16:15:55 +0530 Subject: [PATCH 30/73] fix release note Signed-off-by: Rama Chavali --- changelogs/current.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index aa13162b6c3a..1c6842160dd7 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -242,9 +242,10 @@ new_features: to control writing request and response headers received time in trace output. - area: upstream change: | - Added the ability to specify a custom upstream local address selector using + added the ability to specify a custom upstream local address selector using :ref:`local_address_selector:`. - area: composite filter + change: | added :ref:`ExtensionConfiguration discovery service` support for :ref:`composite filter `. From b62c7aa68df1bafe073f737790d641d9ef7acacb Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 12 Sep 2023 17:32:24 +0530 Subject: [PATCH 31/73] fix compile time options test Signed-off-by: Rama Chavali --- .../filters/http/composite/composite_filter_integration_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index 4764873c2b56..bcc9c826c651 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -146,6 +146,7 @@ class CompositeFilterIntegrationTest : public testing::TestWithParam Date: Mon, 18 Sep 2023 09:25:38 +0530 Subject: [PATCH 32/73] remove oneof Signed-off-by: Rama Chavali --- .../filters/http/composite/v3/composite.proto | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index 1e87a527de7e..6096347e060b 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -48,14 +48,13 @@ message DynamicConfig { // Composite match action (see :ref:`matching docs ` for more info on match actions). // This specifies the filter configuration of the filter that the composite filter should delegate filter interactions to. message ExecuteFilterAction { - oneof config_type { - // Filter specific configuration which depends on the filter being - // instantiated. See the supported filters for further documentation. - // [#extension-category: envoy.filters.listener,envoy.filters.udp_listener] - config.core.v3.TypedExtensionConfig typed_config = 1; - - // Dynamic configuration of filter obtained via extension configuration discovery - // service . - DynamicConfig dynamic_config = 2; - } + // Filter specific configuration which depends on the filter being + // instantiated. See the supported filters for further documentation. + // [#extension-category: envoy.filters.listener,envoy.filters.udp_listener] + config.core.v3.TypedExtensionConfig typed_config = 1 + [(udpa.annotations.field_migrate).oneof_promotion = "config_type"]; + + // Dynamic configuration of filter obtained via extension configuration discovery + // service . + DynamicConfig dynamic_config = 2; } From 0d855e78ec22e2163bda6e2be184757c48d96532 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Fri, 29 Sep 2023 14:21:13 +0530 Subject: [PATCH 33/73] simplify interface and remove local storage Signed-off-by: Rama Chavali --- .../filters/http/composite/v3/composite.proto | 1 + .../dynamic_extension_config_provider.h | 1 - envoy/http/filter_factory.h | 8 +++++ envoy/server/BUILD | 1 + envoy/server/factory_context.h | 21 ++++--------- source/common/filter/config_discovery_impl.h | 16 +++------- source/common/http/filter_chain_helper.cc | 2 +- source/common/http/filter_chain_helper.h | 10 +++---- .../filters/http/composite/action.cc | 17 +++++------ .../network/http_connection_manager/config.h | 4 +-- .../filter_chain_manager_impl.cc | 22 ++++---------- .../filter_chain_manager_impl.h | 14 ++------- .../listener_manager/listener_impl.cc | 30 +++++++------------ .../listener_manager/listener_impl.h | 18 +++-------- source/server/factory_context_impl.cc | 11 +++---- source/server/factory_context_impl.h | 9 ++---- .../filter/config_discovery_impl_test.cc | 4 +-- test/common/http/filter_chain_helper_test.cc | 5 ++-- test/mocks/server/factory_context.h | 14 ++++----- .../server/filter_chain_factory_context.h | 14 ++++----- test/mocks/server/listener_factory_context.h | 14 ++++----- 21 files changed, 83 insertions(+), 153 deletions(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index 6096347e060b..7f7ad9ec977e 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -7,6 +7,7 @@ import "envoy/config/core/v3/extension.proto"; import "xds/annotations/v3/status.proto"; +import "udpa/annotations/migrate.proto"; import "udpa/annotations/status.proto"; import "validate/validate.proto"; diff --git a/envoy/config/dynamic_extension_config_provider.h b/envoy/config/dynamic_extension_config_provider.h index a557ad5fc300..4faf1d95b74a 100644 --- a/envoy/config/dynamic_extension_config_provider.h +++ b/envoy/config/dynamic_extension_config_provider.h @@ -51,6 +51,5 @@ class DynamicExtensionConfigProviderBase { template class DynamicExtensionConfigProvider : public DynamicExtensionConfigProviderBase, public ExtensionConfigProvider {}; - } // namespace Config } // namespace Envoy diff --git a/envoy/http/filter_factory.h b/envoy/http/filter_factory.h index 71d6bc51f19f..a6ca0755a079 100644 --- a/envoy/http/filter_factory.h +++ b/envoy/http/filter_factory.h @@ -22,6 +22,14 @@ class FilterChainFactoryCallbacks; */ using FilterFactoryCb = std::function; +// Struct of canonical filter name and HTTP stream filter factory callback. +struct NamedHttpFilterFactoryCb { + // Canonical filter name. + std::string name; + // Factory function used to create filter instances. + Http::FilterFactoryCb factory_cb; +}; + /** * Simple struct of additional contextual information of HTTP filter, e.g. filter config name * from configuration, canonical filter name, etc. diff --git a/envoy/server/BUILD b/envoy/server/BUILD index eb2bd7f99c3e..54469be4902a 100644 --- a/envoy/server/BUILD +++ b/envoy/server/BUILD @@ -184,6 +184,7 @@ envoy_cc_library( ":process_context_interface", "//envoy/access_log:access_log_interface", "//envoy/api:api_interface", + "//envoy/config:dynamic_extension_config_provider_interface", "//envoy/config:typed_config_interface", "//envoy/config:typed_metadata_interface", "//envoy/grpc:context_interface", diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index 7ca3a19e2d2d..504ccf0d2191 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -7,6 +7,7 @@ #include "envoy/access_log/access_log.h" #include "envoy/common/random_generator.h" #include "envoy/config/core/v3/base.pb.h" +#include "envoy/config/dynamic_extension_config_provider.h" #include "envoy/config/typed_config.h" #include "envoy/config/typed_metadata.h" #include "envoy/grpc/context.h" @@ -39,6 +40,9 @@ namespace Envoy { namespace Server { namespace Configuration { +using HttpExtensionConfigProvider = + std::shared_ptr>; + // Shared factory context between server factories and cluster factories class FactoryContextBase { public: @@ -278,30 +282,17 @@ class FactoryContext : public virtual ListenerAccessLogFactoryContext { * the underlying subscriptions to the filter config discovery service. * @param config_source supplies the extension configuration source for the filter configs. * @param filter_config_name the filter config resource name. - * @param factory_context is the context to use for the filter config provider. * @param last_filter_in_filter_chain indicates whether this filter is the last filter in the * configured chain * @param filter_chain_type is the filter chain type * @param listener_filter_matcher is the filter matcher for TCP listener filter. nullptr for other * filter types. */ - virtual void createDynamicFilterConfigProvider( + virtual Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, - Server::Configuration::FactoryContext& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) PURE; - - /** - * Get configuration for dynamic provider. - * @param filter_config_name the filter config resource name. - * - * @return OptRef the filter factory callback. - */ - virtual OptRef - dynamicProviderConfig(const std::string& filter_config_name) PURE; }; /** diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index e284131009b9..ce3147544b12 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -179,20 +179,12 @@ class DynamicFilterConfigProviderImpl : public DynamicFilterConfigProviderImplBa const ProtobufTypes::MessagePtr default_configuration_; }; -// Struct of canonical filter name and HTTP stream filter factory callback. -struct NamedHttpFilterFactoryCb { - // Canonical filter name. - std::string name; - // Factory function used to create filter instances. - Http::FilterFactoryCb factory_cb; -}; - // Implementation of a HTTP dynamic filter config provider. // NeutralHttpFilterConfigFactory can either be a NamedHttpFilterConfigFactory // or an UpstreamHttpFilterConfigFactory. template class HttpDynamicFilterConfigProviderImpl - : public DynamicFilterConfigProviderImpl { + : public DynamicFilterConfigProviderImpl { public: HttpDynamicFilterConfigProviderImpl( FilterConfigSubscriptionSharedPtr& subscription, @@ -216,7 +208,7 @@ class HttpDynamicFilterConfigProviderImpl } private: - NamedHttpFilterFactoryCb + Http::NamedHttpFilterFactoryCb instantiateFilterFactory(const Protobuf::Message& message) const override { auto* factory = Registry::FactoryRegistry::getFactoryByType( message.GetTypeName()); @@ -643,7 +635,7 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa // HTTP filter class HttpFilterConfigProviderManagerImpl : public FilterConfigProviderManagerImpl< - Server::Configuration::NamedHttpFilterConfigFactory, NamedHttpFilterFactoryCb, + Server::Configuration::NamedHttpFilterConfigFactory, Http::NamedHttpFilterFactoryCb, Server::Configuration::FactoryContext, HttpDynamicFilterConfigProviderImpl< Server::Configuration::FactoryContext, @@ -670,7 +662,7 @@ class HttpFilterConfigProviderManagerImpl // HTTP filter class UpstreamHttpFilterConfigProviderManagerImpl : public FilterConfigProviderManagerImpl< - Server::Configuration::UpstreamHttpFilterConfigFactory, NamedHttpFilterFactoryCb, + Server::Configuration::UpstreamHttpFilterConfigFactory, Http::NamedHttpFilterFactoryCb, Server::Configuration::UpstreamFactoryContext, HttpDynamicFilterConfigProviderImpl< Server::Configuration::UpstreamFactoryContext, diff --git a/source/common/http/filter_chain_helper.cc b/source/common/http/filter_chain_helper.cc index 80281d098ca8..ec993f71389e 100644 --- a/source/common/http/filter_chain_helper.cc +++ b/source/common/http/filter_chain_helper.cc @@ -43,7 +43,7 @@ void FilterChainUtility::createFilterChainForFactories( auto config = filter_config_provider->config(); if (config.has_value()) { - Filter::NamedHttpFilterFactoryCb& factory_cb = config.value().get(); + Http::NamedHttpFilterFactoryCb& factory_cb = config.value().get(); manager.applyFilterFactoryCb({filter_config_provider->name(), factory_cb.name}, factory_cb.factory_cb); continue; diff --git a/source/common/http/filter_chain_helper.h b/source/common/http/filter_chain_helper.h index 0f4179c58c1e..4ad357bdfe7b 100644 --- a/source/common/http/filter_chain_helper.h +++ b/source/common/http/filter_chain_helper.h @@ -14,16 +14,16 @@ namespace Envoy { namespace Http { using DownstreamFilterConfigProviderManager = - Filter::FilterConfigProviderManager; using UpstreamFilterConfigProviderManager = - Filter::FilterConfigProviderManager; class FilterChainUtility : Logger::Loggable { public: using FilterFactoriesList = - std::list>; + std::list>; using FiltersList = Protobuf::RepeatedPtrField< envoy::extensions::filters::network::http_connection_manager::v3::HttpFilter>; @@ -49,9 +49,9 @@ template class FilterChainHelper : Logger::Loggable { public: using FilterFactoriesList = - std::list>; + std::list>; using FilterConfigProviderManager = - Filter::FilterConfigProviderManager; + Filter::FilterConfigProviderManager; FilterChainHelper(FilterConfigProviderManager& filter_config_provider_manager, Server::Configuration::ServerFactoryContext& server_context, diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 7e803b98dad8..eb1fcbfec83b 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -17,20 +17,17 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( // Process dynamic filter configuration and setup extension configuration discovery service. if (composite_action.has_dynamic_config()) { auto config_discovery = composite_action.dynamic_config().config_discovery(); - Server::Configuration::ServerFactoryContext& server_factory_context = - context.server_factory_context_.value(); Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); // Create a dynamic filter config provider and register it with the server factory context. - factory_context.createDynamicFilterConfigProvider( - config_discovery, composite_action.dynamic_config().name(), server_factory_context, - factory_context, server_factory_context.clusterManager(), false, "http", nullptr); - return [&factory_context = factory_context, - name = composite_action.dynamic_config().name()]() -> Matcher::ActionPtr { - auto factory_cb = factory_context.dynamicProviderConfig(name); - if (!factory_cb.has_value()) { + auto provider = factory_context.createDynamicFilterConfigProvider( + config_discovery, composite_action.dynamic_config().name(), false, "http", nullptr); + return [provider = std::move(provider)]() -> Matcher::ActionPtr { + auto config_value = provider->config(); + if (!config_value.has_value()) { throw EnvoyException("Failed to get dynamic config for filter"); } - return std::make_unique(factory_cb.value()); + auto factory_cb = config_value.value().get().factory_cb; + return std::make_unique(factory_cb); }; } else { // Static filter configuration. diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index d2c43c2fe142..6350c7cb2b04 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -45,7 +45,7 @@ namespace NetworkFilters { namespace HttpConnectionManager { using FilterConfigProviderManager = - Filter::FilterConfigProviderManager; /** @@ -140,7 +140,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, Http::FilterChainManager& manager, bool = false, const Http::FilterChainOptions& = Http::EmptyFilterChainOptions{}) const override; using FilterFactoriesList = - std::list>; + std::list>; struct FilterConfig { std::unique_ptr filter_factories; bool allow_upgrade; diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc index ec0ce526ee98..1b17beac93b8 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc @@ -194,31 +194,19 @@ bool PerFilterChainFactoryContextImpl::isQuicListener() const { return parent_context_.isQuicListener(); } -void PerFilterChainFactoryContextImpl::createDynamicFilterConfigProvider( +Configuration::HttpExtensionConfigProvider +PerFilterChainFactoryContextImpl::createDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_factory_context, - Server::Configuration::FactoryContext& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) { if (filter_config_provider_manager_ == nullptr) { filter_config_provider_manager_ = std::make_shared(); } - auto provider = filter_config_provider_manager_->createDynamicFilterConfigProvider( - config_source, filter_config_name, server_factory_context, factory_context, cluster_manager, + return filter_config_provider_manager_->createDynamicFilterConfigProvider( + config_source, filter_config_name, getServerFactoryContext(), *this, clusterManager(), last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); - dynamic_providers_[filter_config_name] = std::move(provider); -} - -OptRef -PerFilterChainFactoryContextImpl::dynamicProviderConfig(const std::string& filter_config_name) { - auto provider = dynamic_providers_.find(filter_config_name); - if (provider == dynamic_providers_.end()) { - return absl::nullopt; - } - return provider->second->config()->factory_cb; } FilterChainManagerImpl::FilterChainManagerImpl( diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h index 09780bba10b0..e299321a5562 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h @@ -90,16 +90,11 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; - void createDynamicFilterConfigProvider( + Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, - Server::Configuration::FactoryContext& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - OptRef - dynamicProviderConfig(const std::string& filter_config_name) override; void startDraining() override { is_draining_.store(true); } @@ -113,12 +108,9 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor std::atomic is_draining_{false}; // TODO(ramaraochavali): Move DownstreamFilterConfigProviderManager usages to use this instead of // current singleton. - std::shared_ptr> filter_config_provider_manager_{}; - absl::flat_hash_map>> - dynamic_providers_; }; using FilterChainActionFactoryContext = Configuration::ServerFactoryContext; diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.cc b/source/extensions/listener_managers/listener_manager/listener_impl.cc index 5bfb5f102c72..8d90f297a50a 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_impl.cc @@ -299,15 +299,13 @@ ListenerFactoryContextBaseImpl::getTransportSocketFactoryContext() const { } Stats::Scope& ListenerFactoryContextBaseImpl::listenerScope() { return *listener_scope_; } bool ListenerFactoryContextBaseImpl::isQuicListener() const { return is_quic_; } -void ListenerFactoryContextBaseImpl::createDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, - Server::Configuration::ServerFactoryContext&, Server::Configuration::FactoryContext&, - Upstream::ClusterManager&, bool, const std::string&, - const Network::ListenerFilterMatcherSharedPtr&) {} -OptRef -ListenerFactoryContextBaseImpl::dynamicProviderConfig(const std::string&) { - return absl::nullopt; +Configuration::HttpExtensionConfigProvider +ListenerFactoryContextBaseImpl::createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool, + const std::string&, const Network::ListenerFilterMatcherSharedPtr&) { + return nullptr; } + Network::DrainDecision& ListenerFactoryContextBaseImpl::drainDecision() { return *this; } Server::DrainManager& ListenerFactoryContextBaseImpl::drainManager() { return *drain_manager_; } Init::Manager& ListenerFactoryContextBaseImpl::initManager() { return server_.initManager(); } @@ -961,21 +959,15 @@ Stats::Scope& PerListenerFactoryContextImpl::listenerScope() { bool PerListenerFactoryContextImpl::isQuicListener() const { return listener_factory_context_base_->isQuicListener(); } -void PerListenerFactoryContextImpl::createDynamicFilterConfigProvider( +Configuration::HttpExtensionConfigProvider +PerListenerFactoryContextImpl::createDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, - Server::Configuration::FactoryContext& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) { return listener_factory_context_base_->createDynamicFilterConfigProvider( - config_source, filter_config_name, server_context, factory_context, cluster_manager, - last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); -} -OptRef -PerListenerFactoryContextImpl::dynamicProviderConfig(const std::string& filter_config_name) { - return listener_factory_context_base_->dynamicProviderConfig(filter_config_name); + config_source, filter_config_name, last_filter_in_filter_chain, filter_chain_type, + listener_filter_matcher); } Init::Manager& PerListenerFactoryContextImpl::initManager() { return listener_impl_.initManager(); } diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.h b/source/extensions/listener_managers/listener_manager/listener_impl.h index 65f1f92c35ce..6c604e7564c0 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_impl.h @@ -163,16 +163,11 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; - void createDynamicFilterConfigProvider( + Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, - Server::Configuration::FactoryContext& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - OptRef - dynamicProviderConfig(const std::string& filter_config_name) override; // DrainDecision bool drainClose() const override { @@ -251,16 +246,11 @@ class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryConte Stats::Scope& listenerScope() override; bool isQuicListener() const override; - void createDynamicFilterConfigProvider( + Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, - Server::Configuration::FactoryContext& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - OptRef - dynamicProviderConfig(const std::string& filter_config_name) override; // ListenerFactoryContext const Network::ListenerConfig& listenerConfig() const override; diff --git a/source/server/factory_context_impl.cc b/source/server/factory_context_impl.cc index 8c86ac4397bc..798b91222278 100644 --- a/source/server/factory_context_impl.cc +++ b/source/server/factory_context_impl.cc @@ -61,13 +61,10 @@ envoy::config::core::v3::TrafficDirection FactoryContextImpl::direction() const Network::DrainDecision& FactoryContextImpl::drainDecision() { return drain_decision_; } Stats::Scope& FactoryContextImpl::listenerScope() { return listener_scope_; } bool FactoryContextImpl::isQuicListener() const { return is_quic_; } -void FactoryContextImpl::createDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, - Server::Configuration::ServerFactoryContext&, Server::Configuration::FactoryContext&, - Upstream::ClusterManager&, bool, const std::string&, - const Network::ListenerFilterMatcherSharedPtr&) {} -OptRef FactoryContextImpl::dynamicProviderConfig(const std::string&) { - return absl::nullopt; +Configuration::HttpExtensionConfigProvider FactoryContextImpl::createDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool, + const std::string&, const Network::ListenerFilterMatcherSharedPtr&) { + return nullptr; } } // namespace Server diff --git a/source/server/factory_context_impl.h b/source/server/factory_context_impl.h index 7f7ecd7631b2..c82861757302 100644 --- a/source/server/factory_context_impl.h +++ b/source/server/factory_context_impl.h @@ -47,16 +47,11 @@ class FactoryContextImpl : public Configuration::FactoryContext { Network::DrainDecision& drainDecision() override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; - void createDynamicFilterConfigProvider( + Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, - Server::Configuration::ServerFactoryContext& server_context, - Server::Configuration::FactoryContext& factory_context, - Upstream::ClusterManager& cluster_manager, bool last_filter_in_filter_chain, + const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - OptRef - dynamicProviderConfig(const std::string& filter_config_name) override; private: Server::Instance& server_; diff --git a/test/common/filter/config_discovery_impl_test.cc b/test/common/filter/config_discovery_impl_test.cc index 1071162b9553..f6eabd01dcb2 100644 --- a/test/common/filter/config_discovery_impl_test.cc +++ b/test/common/filter/config_discovery_impl_test.cc @@ -276,7 +276,7 @@ class FilterConfigDiscoveryImplTest : public FilterConfigDiscoveryTestBase { // HTTP filter test class HttpFilterConfigDiscoveryImplTest : public FilterConfigDiscoveryImplTest< - NamedHttpFilterFactoryCb, Server::Configuration::FactoryContext, + Http::NamedHttpFilterFactoryCb, Server::Configuration::FactoryContext, HttpFilterConfigProviderManagerImpl, TestHttpFilterFactory, Server::Configuration::NamedHttpFilterConfigFactory, Server::Configuration::MockFactoryContext> { @@ -293,7 +293,7 @@ class HttpFilterConfigDiscoveryImplTest // HTTP upstream filter test class HttpUpstreamFilterConfigDiscoveryImplTest : public FilterConfigDiscoveryImplTest< - NamedHttpFilterFactoryCb, Server::Configuration::UpstreamFactoryContext, + Http::NamedHttpFilterFactoryCb, Server::Configuration::UpstreamFactoryContext, UpstreamHttpFilterConfigProviderManagerImpl, TestHttpFilterFactory, Server::Configuration::UpstreamHttpFilterConfigFactory, Server::Configuration::MockUpstreamFactoryContext> { diff --git a/test/common/http/filter_chain_helper_test.cc b/test/common/http/filter_chain_helper_test.cc index dfeefcde2907..b89b478a0ea2 100644 --- a/test/common/http/filter_chain_helper_test.cc +++ b/test/common/http/filter_chain_helper_test.cc @@ -28,9 +28,8 @@ TEST(FilterChainUtilityTest, CreateFilterChainForFactoriesWithRouteDisabled) { for (const auto& name : {"filter_0", "filter_1", "filter_2"}) { auto provider = - std::make_unique>( - Filter::NamedHttpFilterFactoryCb{"filter_type_name", - [](FilterChainFactoryCallbacks&) {}}, + std::make_unique>( + Http::NamedHttpFilterFactoryCb{"filter_type_name", [](FilterChainFactoryCallbacks&) {}}, name); filter_factories.push_back(std::move(provider)); } diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index f8a002dc0f41..2226eca37b15 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -57,15 +57,11 @@ class MockFactoryContext : public virtual ListenerFactoryContext { MOCK_METHOD(ProtobufMessage::ValidationVisitor&, messageValidationVisitor, ()); MOCK_METHOD(Api::Api&, api, ()); - void createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, - Server::Configuration::ServerFactoryContext&, - Server::Configuration::FactoryContext&, - Upstream::ClusterManager&, bool, const std::string&, - const Network::ListenerFilterMatcherSharedPtr&) override {} - - OptRef dynamicProviderConfig(const std::string&) override { - return absl::nullopt; + HttpExtensionConfigProvider + createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, + const std::string&, bool, const std::string&, + const Network::ListenerFilterMatcherSharedPtr&) override { + return nullptr; } testing::NiceMock server_factory_context_; diff --git a/test/mocks/server/filter_chain_factory_context.h b/test/mocks/server/filter_chain_factory_context.h index 451a69167f44..f14ec2d8eaf4 100644 --- a/test/mocks/server/filter_chain_factory_context.h +++ b/test/mocks/server/filter_chain_factory_context.h @@ -12,15 +12,11 @@ class MockFilterChainFactoryContext : public MockFactoryContext, public FilterCh MockFilterChainFactoryContext(); ~MockFilterChainFactoryContext() override; - void createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, - Server::Configuration::ServerFactoryContext&, - Server::Configuration::FactoryContext&, - Upstream::ClusterManager&, bool, const std::string&, - const Network::ListenerFilterMatcherSharedPtr&) override {} - - OptRef dynamicProviderConfig(const std::string&) override { - return absl::nullopt; + HttpExtensionConfigProvider + createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, + const std::string&, bool, const std::string&, + const Network::ListenerFilterMatcherSharedPtr&) override { + return nullptr; } }; } // namespace Configuration diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index 83b55939d1f8..aae9ec3f9eb8 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -56,15 +56,11 @@ class MockListenerFactoryContext : public ListenerFactoryContext { MOCK_METHOD(ProtobufMessage::ValidationVisitor&, messageValidationVisitor, ()); MOCK_METHOD(Api::Api&, api, ()); - void createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, - Server::Configuration::ServerFactoryContext&, - Server::Configuration::FactoryContext&, - Upstream::ClusterManager&, bool, const std::string&, - const Network::ListenerFilterMatcherSharedPtr&) override {} - - OptRef dynamicProviderConfig(const std::string&) override { - return absl::nullopt; + HttpExtensionConfigProvider + createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, + const std::string&, bool, const std::string&, + const Network::ListenerFilterMatcherSharedPtr&) override { + return nullptr; } testing::NiceMock server_factory_context_; From 48a053476a087fd947d90d3a67b86f4b5ef81036 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Fri, 29 Sep 2023 14:43:49 +0530 Subject: [PATCH 34/73] resolve conflicts Signed-off-by: Rama Chavali --- .../extensions/filters/http/composite/v3/composite.proto | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index 0a3f552b5e59..4278a58c9b8c 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -5,13 +5,9 @@ package envoy.extensions.filters.http.composite.v3; import "envoy/config/core/v3/config_source.proto"; import "envoy/config/core/v3/extension.proto"; -<<<<<<< HEAD -import "xds/annotations/v3/status.proto"; - import "udpa/annotations/migrate.proto"; -======= ->>>>>>> upstream/main import "udpa/annotations/status.proto"; + import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.http.composite.v3"; From 4399275d247e71ced7cc7506cdcb4e3bac5845c7 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Fri, 29 Sep 2023 16:53:52 +0530 Subject: [PATCH 35/73] wire up downstream filter provider Signed-off-by: Rama Chavali --- .../filters/http/composite/v3/composite.proto | 1 - envoy/server/factory_context.h | 18 ++++++++++++++++++ .../filter_chain_manager_impl.cc | 12 +++++++----- .../filter_chain_manager_impl.h | 8 ++------ .../listener_manager/listener_impl.cc | 8 ++++++++ .../listener_manager/listener_impl.h | 2 ++ source/server/factory_context_impl.cc | 3 +++ source/server/factory_context_impl.h | 2 +- test/mocks/server/factory_context.h | 3 +++ .../server/filter_chain_factory_context.h | 3 +++ test/mocks/server/listener_factory_context.h | 4 ++++ 11 files changed, 51 insertions(+), 13 deletions(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index 4278a58c9b8c..c6fefc7b1512 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -7,7 +7,6 @@ import "envoy/config/core/v3/extension.proto"; import "udpa/annotations/migrate.proto"; import "udpa/annotations/status.proto"; - import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.http.composite.v3"; diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index 504ccf0d2191..7ec01c1f0784 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -37,6 +37,9 @@ #include "source/common/protobuf/protobuf.h" namespace Envoy { + namespace Filter { + template class FilterConfigProviderManager; + } // namespace Filter namespace Server { namespace Configuration { @@ -210,6 +213,12 @@ class ListenerAccessLogFactoryContext : public virtual CommonFactoryContext { virtual ProcessContextOptRef processContext() PURE; }; +class FactoryContext; + +using DownstreamFilterConfigProviderManager = + Filter::FilterConfigProviderManager; +using DownstreamFilterConfigProviderManagerPtr = std::shared_ptr; /** * Context passed to network and HTTP filters to access server resources. * TODO(mattklein123): When we lock down visibility of the rest of the code, filters should only @@ -287,12 +296,21 @@ class FactoryContext : public virtual ListenerAccessLogFactoryContext { * @param filter_chain_type is the filter chain type * @param listener_filter_matcher is the filter matcher for TCP listener filter. nullptr for other * filter types. + * + * @return HttpExtensionConfigProvider */ virtual Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) PURE; + + /** + * Returns the downstream filter config provider manager. + * + * @return DownstreamFilterConfigProviderManagerPtr + */ + virtual DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() PURE; }; /** diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc index 1b17beac93b8..e6b42182cb45 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc @@ -79,7 +79,8 @@ PerFilterChainFactoryContextImpl::PerFilterChainFactoryContextImpl( Configuration::FactoryContext& parent_context, Init::Manager& init_manager) : parent_context_(parent_context), scope_(parent_context_.scope().createScope("")), filter_chain_scope_(parent_context_.listenerScope().createScope("")), - init_manager_(init_manager) {} + init_manager_(init_manager),filter_config_provider_manager_(std::make_shared()) { + } bool PerFilterChainFactoryContextImpl::drainClose() const { return is_draining_.load() || parent_context_.drainDecision().drainClose(); @@ -200,15 +201,16 @@ PerFilterChainFactoryContextImpl::createDynamicFilterConfigProvider( const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) { - if (filter_config_provider_manager_ == nullptr) { - filter_config_provider_manager_ = - std::make_shared(); - } return filter_config_provider_manager_->createDynamicFilterConfigProvider( config_source, filter_config_name, getServerFactoryContext(), *this, clusterManager(), last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); } +Configuration::DownstreamFilterConfigProviderManagerPtr PerFilterChainFactoryContextImpl::downstreamFilterConfigProviderManager() { + return filter_config_provider_manager_; +} + + FilterChainManagerImpl::FilterChainManagerImpl( const std::vector& addresses, Configuration::FactoryContext& factory_context, Init::Manager& init_manager, diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h index e299321a5562..db99b510f29c 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h @@ -95,7 +95,7 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - + Configuration::DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() override; void startDraining() override { is_draining_.store(true); } private: @@ -106,11 +106,7 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor Stats::ScopeSharedPtr filter_chain_scope_; Init::Manager& init_manager_; std::atomic is_draining_{false}; - // TODO(ramaraochavali): Move DownstreamFilterConfigProviderManager usages to use this instead of - // current singleton. - std::shared_ptr> - filter_config_provider_manager_{}; + Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager_{}; }; using FilterChainActionFactoryContext = Configuration::ServerFactoryContext; diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.cc b/source/extensions/listener_managers/listener_manager/listener_impl.cc index 96d11d032e5a..7a23a09358e8 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_impl.cc @@ -305,6 +305,10 @@ ListenerFactoryContextBaseImpl::createDynamicFilterConfigProvider( const std::string&, const Network::ListenerFilterMatcherSharedPtr&) { return nullptr; } +Configuration::DownstreamFilterConfigProviderManagerPtr ListenerFactoryContextBaseImpl::downstreamFilterConfigProviderManager() { + return nullptr; +} + Network::DrainDecision& ListenerFactoryContextBaseImpl::drainDecision() { return *this; } Server::DrainManager& ListenerFactoryContextBaseImpl::drainManager() { return *drain_manager_; } @@ -969,6 +973,10 @@ PerListenerFactoryContextImpl::createDynamicFilterConfigProvider( config_source, filter_config_name, last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); } + Configuration::DownstreamFilterConfigProviderManagerPtr PerListenerFactoryContextImpl::downstreamFilterConfigProviderManager() { + return listener_factory_context_base_->downstreamFilterConfigProviderManager(); + } + Init::Manager& PerListenerFactoryContextImpl::initManager() { return listener_impl_.initManager(); } bool ListenerImpl::createNetworkFilterChain( diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.h b/source/extensions/listener_managers/listener_manager/listener_impl.h index 40017a7203b3..ca3b495a818b 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_impl.h @@ -165,6 +165,7 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; + Configuration::DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() override; // DrainDecision bool drainClose() const override { @@ -248,6 +249,7 @@ class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryConte const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; + Configuration::DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() override; // ListenerFactoryContext const Network::ListenerConfig& listenerConfig() const override; diff --git a/source/server/factory_context_impl.cc b/source/server/factory_context_impl.cc index 798b91222278..a15a5f217c31 100644 --- a/source/server/factory_context_impl.cc +++ b/source/server/factory_context_impl.cc @@ -66,6 +66,9 @@ Configuration::HttpExtensionConfigProvider FactoryContextImpl::createDynamicFilt const std::string&, const Network::ListenerFilterMatcherSharedPtr&) { return nullptr; } + Configuration::DownstreamFilterConfigProviderManagerPtr FactoryContextImpl::downstreamFilterConfigProviderManager() { + return nullptr; + } } // namespace Server } // namespace Envoy diff --git a/source/server/factory_context_impl.h b/source/server/factory_context_impl.h index c82861757302..ab562c380c7e 100644 --- a/source/server/factory_context_impl.h +++ b/source/server/factory_context_impl.h @@ -52,7 +52,7 @@ class FactoryContextImpl : public Configuration::FactoryContext { const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - +Configuration::DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() override; private: Server::Instance& server_; const envoy::config::listener::v3::Listener& config_; diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index 2226eca37b15..020b0235fc6f 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -63,6 +63,9 @@ class MockFactoryContext : public virtual ListenerFactoryContext { const Network::ListenerFilterMatcherSharedPtr&) override { return nullptr; } + Configuration::DownstreamFilterConfigProviderManagerPtr FactoryContextImpl::downstreamFilterConfigProviderManager() override { + return nullptr; + } testing::NiceMock server_factory_context_; testing::NiceMock access_log_manager_; diff --git a/test/mocks/server/filter_chain_factory_context.h b/test/mocks/server/filter_chain_factory_context.h index f14ec2d8eaf4..aeb86f397e6f 100644 --- a/test/mocks/server/filter_chain_factory_context.h +++ b/test/mocks/server/filter_chain_factory_context.h @@ -18,6 +18,9 @@ class MockFilterChainFactoryContext : public MockFactoryContext, public FilterCh const Network::ListenerFilterMatcherSharedPtr&) override { return nullptr; } + Configuration::DownstreamFilterConfigProviderManagerPtr FactoryContextImpl::downstreamFilterConfigProviderManager() override { + return nullptr; + } }; } // namespace Configuration } // namespace Server diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index aae9ec3f9eb8..1f52b214a266 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -63,6 +63,10 @@ class MockListenerFactoryContext : public ListenerFactoryContext { return nullptr; } + Configuration::DownstreamFilterConfigProviderManagerPtr FactoryContextImpl::downstreamFilterConfigProviderManager() override { + return nullptr; + } + testing::NiceMock server_factory_context_; testing::NiceMock access_log_manager_; testing::NiceMock cluster_manager_; From dfd597ad66b831de85b84ad6d8b09b4f0f523d00 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Fri, 29 Sep 2023 17:10:21 +0530 Subject: [PATCH 36/73] remove singleton downstream provider Signed-off-by: Rama Chavali --- envoy/server/factory_context.h | 15 ++++++++------- source/common/http/filter_chain_helper.cc | 12 ------------ source/common/http/filter_chain_helper.h | 7 ------- .../network/http_connection_manager/config.cc | 5 ++--- .../listener_manager/filter_chain_manager_impl.cc | 9 +++++---- .../listener_manager/filter_chain_manager_impl.h | 3 ++- .../listener_manager/listener_impl.cc | 9 +++++---- .../listener_manager/listener_impl.h | 6 ++++-- source/server/factory_context_impl.cc | 7 ++++--- source/server/factory_context_impl.h | 4 +++- test/mocks/server/factory_context.h | 3 ++- test/mocks/server/filter_chain_factory_context.h | 3 ++- test/mocks/server/listener_factory_context.h | 3 ++- 13 files changed, 39 insertions(+), 47 deletions(-) diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index 7ec01c1f0784..9898f7cf05e9 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -37,9 +37,9 @@ #include "source/common/protobuf/protobuf.h" namespace Envoy { - namespace Filter { - template class FilterConfigProviderManager; - } // namespace Filter +namespace Filter { +template class FilterConfigProviderManager; +} // namespace Filter namespace Server { namespace Configuration { @@ -218,7 +218,8 @@ class FactoryContext; using DownstreamFilterConfigProviderManager = Filter::FilterConfigProviderManager; -using DownstreamFilterConfigProviderManagerPtr = std::shared_ptr; +using DownstreamFilterConfigProviderManagerPtr = + std::shared_ptr; /** * Context passed to network and HTTP filters to access server resources. * TODO(mattklein123): When we lock down visibility of the rest of the code, filters should only @@ -296,7 +297,7 @@ class FactoryContext : public virtual ListenerAccessLogFactoryContext { * @param filter_chain_type is the filter chain type * @param listener_filter_matcher is the filter matcher for TCP listener filter. nullptr for other * filter types. - * + * * @return HttpExtensionConfigProvider */ virtual Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( @@ -307,8 +308,8 @@ class FactoryContext : public virtual ListenerAccessLogFactoryContext { /** * Returns the downstream filter config provider manager. - * - * @return DownstreamFilterConfigProviderManagerPtr + * + * @return DownstreamFilterConfigProviderManagerPtr */ virtual DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() PURE; }; diff --git a/source/common/http/filter_chain_helper.cc b/source/common/http/filter_chain_helper.cc index c26d2cc99c11..77f20b250bf8 100644 --- a/source/common/http/filter_chain_helper.cc +++ b/source/common/http/filter_chain_helper.cc @@ -60,7 +60,6 @@ void FilterChainUtility::createFilterChainForFactories( } } -SINGLETON_MANAGER_REGISTRATION(downstream_filter_config_provider_manager); SINGLETON_MANAGER_REGISTRATION(upstream_filter_config_provider_manager); std::shared_ptr @@ -73,16 +72,5 @@ FilterChainUtility::createSingletonUpstreamFilterConfigProviderManager( return upstream_filter_config_provider_manager; } -std::shared_ptr -FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( - Server::Configuration::ServerFactoryContext& context) { - std::shared_ptr - downstream_filter_config_provider_manager = - context.singletonManager().getTyped( - SINGLETON_MANAGER_REGISTERED_NAME(downstream_filter_config_provider_manager), - [] { return std::make_shared(); }); - return downstream_filter_config_provider_manager; -} - } // namespace Http } // namespace Envoy diff --git a/source/common/http/filter_chain_helper.h b/source/common/http/filter_chain_helper.h index d125dabe8899..b7a3e60cc639 100644 --- a/source/common/http/filter_chain_helper.h +++ b/source/common/http/filter_chain_helper.h @@ -13,9 +13,6 @@ namespace Envoy { namespace Http { -using DownstreamFilterConfigProviderManager = - Filter::FilterConfigProviderManager; using UpstreamFilterConfigProviderManager = Filter::FilterConfigProviderManager; @@ -31,10 +28,6 @@ class FilterChainUtility : Logger::Loggable { const FilterChainOptions& options, const FilterFactoriesList& filter_factories); - static std::shared_ptr - createSingletonDownstreamFilterConfigProviderManager( - Server::Configuration::ServerFactoryContext& context); - static std::shared_ptr createSingletonUpstreamFilterConfigProviderManager( Server::Configuration::ServerFactoryContext& context); diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index bf9e54ad3393..d3e39457a375 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -230,9 +230,8 @@ Utility::Singletons Utility::createSingletons(Server::Configuration::FactoryCont auto tracer_manager = Tracing::TracerManagerImpl::singleton(context); - std::shared_ptr filter_config_provider_manager = - Http::FilterChainUtility::createSingletonDownstreamFilterConfigProviderManager( - context.getServerFactoryContext()); + std::shared_ptr + filter_config_provider_manager = context.downstreamFilterConfigProviderManager(); return {date_provider, route_config_provider_manager, scoped_routes_config_provider_manager, tracer_manager, filter_config_provider_manager}; diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc index e6b42182cb45..81903a44a99f 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc @@ -79,8 +79,9 @@ PerFilterChainFactoryContextImpl::PerFilterChainFactoryContextImpl( Configuration::FactoryContext& parent_context, Init::Manager& init_manager) : parent_context_(parent_context), scope_(parent_context_.scope().createScope("")), filter_chain_scope_(parent_context_.listenerScope().createScope("")), - init_manager_(init_manager),filter_config_provider_manager_(std::make_shared()) { - } + init_manager_(init_manager), + filter_config_provider_manager_( + std::make_shared()) {} bool PerFilterChainFactoryContextImpl::drainClose() const { return is_draining_.load() || parent_context_.drainDecision().drainClose(); @@ -206,11 +207,11 @@ PerFilterChainFactoryContextImpl::createDynamicFilterConfigProvider( last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); } -Configuration::DownstreamFilterConfigProviderManagerPtr PerFilterChainFactoryContextImpl::downstreamFilterConfigProviderManager() { +Configuration::DownstreamFilterConfigProviderManagerPtr +PerFilterChainFactoryContextImpl::downstreamFilterConfigProviderManager() { return filter_config_provider_manager_; } - FilterChainManagerImpl::FilterChainManagerImpl( const std::vector& addresses, Configuration::FactoryContext& factory_context, Init::Manager& init_manager, diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h index db99b510f29c..78d78ab47911 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h @@ -95,7 +95,8 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - Configuration::DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() override; + Configuration::DownstreamFilterConfigProviderManagerPtr + downstreamFilterConfigProviderManager() override; void startDraining() override { is_draining_.store(true); } private: diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.cc b/source/extensions/listener_managers/listener_manager/listener_impl.cc index 7a23a09358e8..d6e67fbf3b59 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_impl.cc @@ -305,11 +305,11 @@ ListenerFactoryContextBaseImpl::createDynamicFilterConfigProvider( const std::string&, const Network::ListenerFilterMatcherSharedPtr&) { return nullptr; } -Configuration::DownstreamFilterConfigProviderManagerPtr ListenerFactoryContextBaseImpl::downstreamFilterConfigProviderManager() { +Configuration::DownstreamFilterConfigProviderManagerPtr +ListenerFactoryContextBaseImpl::downstreamFilterConfigProviderManager() { return nullptr; } - Network::DrainDecision& ListenerFactoryContextBaseImpl::drainDecision() { return *this; } Server::DrainManager& ListenerFactoryContextBaseImpl::drainManager() { return *drain_manager_; } Init::Manager& ListenerFactoryContextBaseImpl::initManager() { return server_.initManager(); } @@ -973,9 +973,10 @@ PerListenerFactoryContextImpl::createDynamicFilterConfigProvider( config_source, filter_config_name, last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); } - Configuration::DownstreamFilterConfigProviderManagerPtr PerListenerFactoryContextImpl::downstreamFilterConfigProviderManager() { +Configuration::DownstreamFilterConfigProviderManagerPtr +PerListenerFactoryContextImpl::downstreamFilterConfigProviderManager() { return listener_factory_context_base_->downstreamFilterConfigProviderManager(); - } +} Init::Manager& PerListenerFactoryContextImpl::initManager() { return listener_impl_.initManager(); } diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.h b/source/extensions/listener_managers/listener_manager/listener_impl.h index ca3b495a818b..0d89f5d3ac25 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_impl.h @@ -165,7 +165,8 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - Configuration::DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() override; + Configuration::DownstreamFilterConfigProviderManagerPtr + downstreamFilterConfigProviderManager() override; // DrainDecision bool drainClose() const override { @@ -249,7 +250,8 @@ class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryConte const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - Configuration::DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() override; + Configuration::DownstreamFilterConfigProviderManagerPtr + downstreamFilterConfigProviderManager() override; // ListenerFactoryContext const Network::ListenerConfig& listenerConfig() const override; diff --git a/source/server/factory_context_impl.cc b/source/server/factory_context_impl.cc index a15a5f217c31..b1bef5b30761 100644 --- a/source/server/factory_context_impl.cc +++ b/source/server/factory_context_impl.cc @@ -66,9 +66,10 @@ Configuration::HttpExtensionConfigProvider FactoryContextImpl::createDynamicFilt const std::string&, const Network::ListenerFilterMatcherSharedPtr&) { return nullptr; } - Configuration::DownstreamFilterConfigProviderManagerPtr FactoryContextImpl::downstreamFilterConfigProviderManager() { - return nullptr; - } +Configuration::DownstreamFilterConfigProviderManagerPtr +FactoryContextImpl::downstreamFilterConfigProviderManager() { + return nullptr; +} } // namespace Server } // namespace Envoy diff --git a/source/server/factory_context_impl.h b/source/server/factory_context_impl.h index ab562c380c7e..105d47052405 100644 --- a/source/server/factory_context_impl.h +++ b/source/server/factory_context_impl.h @@ -52,7 +52,9 @@ class FactoryContextImpl : public Configuration::FactoryContext { const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; -Configuration::DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() override; + Configuration::DownstreamFilterConfigProviderManagerPtr + downstreamFilterConfigProviderManager() override; + private: Server::Instance& server_; const envoy::config::listener::v3::Listener& config_; diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index 020b0235fc6f..d957fb235642 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -63,7 +63,8 @@ class MockFactoryContext : public virtual ListenerFactoryContext { const Network::ListenerFilterMatcherSharedPtr&) override { return nullptr; } - Configuration::DownstreamFilterConfigProviderManagerPtr FactoryContextImpl::downstreamFilterConfigProviderManager() override { + Configuration::DownstreamFilterConfigProviderManagerPtr + FactoryContextImpl::downstreamFilterConfigProviderManager() override { return nullptr; } diff --git a/test/mocks/server/filter_chain_factory_context.h b/test/mocks/server/filter_chain_factory_context.h index aeb86f397e6f..997a58db992e 100644 --- a/test/mocks/server/filter_chain_factory_context.h +++ b/test/mocks/server/filter_chain_factory_context.h @@ -18,7 +18,8 @@ class MockFilterChainFactoryContext : public MockFactoryContext, public FilterCh const Network::ListenerFilterMatcherSharedPtr&) override { return nullptr; } - Configuration::DownstreamFilterConfigProviderManagerPtr FactoryContextImpl::downstreamFilterConfigProviderManager() override { + Configuration::DownstreamFilterConfigProviderManagerPtr + FactoryContextImpl::downstreamFilterConfigProviderManager() override { return nullptr; } }; diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index 1f52b214a266..e96f99030132 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -63,7 +63,8 @@ class MockListenerFactoryContext : public ListenerFactoryContext { return nullptr; } - Configuration::DownstreamFilterConfigProviderManagerPtr FactoryContextImpl::downstreamFilterConfigProviderManager() override { + Configuration::DownstreamFilterConfigProviderManagerPtr + FactoryContextImpl::downstreamFilterConfigProviderManager() override { return nullptr; } From bf4074ea5f3b5a97c3de2bc5f751d67aef767f30 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Fri, 29 Sep 2023 19:07:06 +0530 Subject: [PATCH 37/73] fix test compile Signed-off-by: Rama Chavali --- test/mocks/server/filter_chain_factory_context.h | 2 +- test/mocks/server/listener_factory_context.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/mocks/server/filter_chain_factory_context.h b/test/mocks/server/filter_chain_factory_context.h index 997a58db992e..b5ab43a657ec 100644 --- a/test/mocks/server/filter_chain_factory_context.h +++ b/test/mocks/server/filter_chain_factory_context.h @@ -19,7 +19,7 @@ class MockFilterChainFactoryContext : public MockFactoryContext, public FilterCh return nullptr; } Configuration::DownstreamFilterConfigProviderManagerPtr - FactoryContextImpl::downstreamFilterConfigProviderManager() override { + downstreamFilterConfigProviderManager() override { return nullptr; } }; diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index e96f99030132..3a025b6030cb 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -64,7 +64,7 @@ class MockListenerFactoryContext : public ListenerFactoryContext { } Configuration::DownstreamFilterConfigProviderManagerPtr - FactoryContextImpl::downstreamFilterConfigProviderManager() override { + downstreamFilterConfigProviderManager() override { return nullptr; } From c0a5dc140ec5972d8ff9fc5f926494256d809930 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Fri, 29 Sep 2023 19:17:51 +0530 Subject: [PATCH 38/73] fix test compile Signed-off-by: Rama Chavali --- test/mocks/server/factory_context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index d957fb235642..b9995c44f7a2 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -64,7 +64,7 @@ class MockFactoryContext : public virtual ListenerFactoryContext { return nullptr; } Configuration::DownstreamFilterConfigProviderManagerPtr - FactoryContextImpl::downstreamFilterConfigProviderManager() override { + downstreamFilterConfigProviderManager() override { return nullptr; } From 1fdc2359a5bb767fc87dd64b415b2ad9a21d4848 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sat, 30 Sep 2023 09:12:07 +0530 Subject: [PATCH 39/73] fix api listener Signed-off-by: Rama Chavali --- source/server/BUILD | 1 + source/server/factory_context_impl.cc | 6 ++++-- source/server/factory_context_impl.h | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/source/server/BUILD b/source/server/BUILD index 582a56c00ee1..367cf7b5795c 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -327,6 +327,7 @@ envoy_cc_library( deps = [ "//envoy/server:factory_context_interface", "//envoy/server:instance_interface", + "//source/common/filter:config_discovery_lib", ], ) diff --git a/source/server/factory_context_impl.cc b/source/server/factory_context_impl.cc index b1bef5b30761..77e17e906fe2 100644 --- a/source/server/factory_context_impl.cc +++ b/source/server/factory_context_impl.cc @@ -9,7 +9,9 @@ FactoryContextImpl::FactoryContextImpl(Server::Instance& server, Stats::Scope& global_scope, Stats::Scope& listener_scope, bool is_quic) : server_(server), config_(config), drain_decision_(drain_decision), - global_scope_(global_scope), listener_scope_(listener_scope), is_quic_(is_quic) {} + global_scope_(global_scope), listener_scope_(listener_scope), is_quic_(is_quic), + filter_config_provider_manager_( + std::make_shared()) {} AccessLog::AccessLogManager& FactoryContextImpl::accessLogManager() { return server_.accessLogManager(); @@ -68,7 +70,7 @@ Configuration::HttpExtensionConfigProvider FactoryContextImpl::createDynamicFilt } Configuration::DownstreamFilterConfigProviderManagerPtr FactoryContextImpl::downstreamFilterConfigProviderManager() { - return nullptr; + return filter_config_provider_manager_; } } // namespace Server diff --git a/source/server/factory_context_impl.h b/source/server/factory_context_impl.h index 105d47052405..58abc9973059 100644 --- a/source/server/factory_context_impl.h +++ b/source/server/factory_context_impl.h @@ -3,6 +3,8 @@ #include "envoy/server/factory_context.h" #include "envoy/server/instance.h" +#include "source/common/filter/config_discovery_impl.h" + namespace Envoy { namespace Server { @@ -62,6 +64,7 @@ class FactoryContextImpl : public Configuration::FactoryContext { Stats::Scope& global_scope_; Stats::Scope& listener_scope_; bool is_quic_; + Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager_{}; }; } // namespace Server From 7da1aaf596a1502849a5a00bcd1ae0f340c38cae Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sun, 1 Oct 2023 12:45:48 +0530 Subject: [PATCH 40/73] move providers to listener manager Signed-off-by: Rama Chavali --- .../filter_chain_manager_impl.cc | 8 +++---- .../filter_chain_manager_impl.h | 1 - .../listener_manager/listener_impl.cc | 11 +++++---- .../listener_manager/listener_impl.h | 24 +++++++++++-------- .../listener_manager/listener_manager_impl.cc | 2 ++ .../listener_manager/listener_manager_impl.h | 1 + 6 files changed, 27 insertions(+), 20 deletions(-) diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc index 81903a44a99f..40d23906a896 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc @@ -79,9 +79,7 @@ PerFilterChainFactoryContextImpl::PerFilterChainFactoryContextImpl( Configuration::FactoryContext& parent_context, Init::Manager& init_manager) : parent_context_(parent_context), scope_(parent_context_.scope().createScope("")), filter_chain_scope_(parent_context_.listenerScope().createScope("")), - init_manager_(init_manager), - filter_config_provider_manager_( - std::make_shared()) {} + init_manager_(init_manager) {} bool PerFilterChainFactoryContextImpl::drainClose() const { return is_draining_.load() || parent_context_.drainDecision().drainClose(); @@ -202,14 +200,14 @@ PerFilterChainFactoryContextImpl::createDynamicFilterConfigProvider( const std::string& filter_config_name, bool last_filter_in_filter_chain, const std::string& filter_chain_type, const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) { - return filter_config_provider_manager_->createDynamicFilterConfigProvider( + return downstreamFilterConfigProviderManager()->createDynamicFilterConfigProvider( config_source, filter_config_name, getServerFactoryContext(), *this, clusterManager(), last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); } Configuration::DownstreamFilterConfigProviderManagerPtr PerFilterChainFactoryContextImpl::downstreamFilterConfigProviderManager() { - return filter_config_provider_manager_; + return parent_context_.downstreamFilterConfigProviderManager(); } FilterChainManagerImpl::FilterChainManagerImpl( diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h index 78d78ab47911..3c4d017fc717 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h @@ -107,7 +107,6 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor Stats::ScopeSharedPtr filter_chain_scope_; Init::Manager& init_manager_; std::atomic is_draining_{false}; - Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager_{}; }; using FilterChainActionFactoryContext = Configuration::ServerFactoryContext; diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.cc b/source/extensions/listener_managers/listener_manager/listener_impl.cc index d6e67fbf3b59..574627aebe05 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_impl.cc @@ -229,13 +229,15 @@ std::string listenerStatsScope(const envoy::config::listener::v3::Listener& conf ListenerFactoryContextBaseImpl::ListenerFactoryContextBaseImpl( Envoy::Server::Instance& server, ProtobufMessage::ValidationVisitor& validation_visitor, - const envoy::config::listener::v3::Listener& config, DrainManagerPtr drain_manager) + const envoy::config::listener::v3::Listener& config, DrainManagerPtr drain_manager, + Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager) : server_(server), metadata_(config.metadata()), typed_metadata_(config.metadata()), direction_(config.traffic_direction()), global_scope_(server.stats().createScope("")), listener_scope_( server_.stats().createScope(fmt::format("listener.{}.", listenerStatsScope(config)))), validation_visitor_(validation_visitor), drain_manager_(std::move(drain_manager)), - is_quic_(config.udp_listener_config().has_quic_options()) {} + is_quic_(config.udp_listener_config().has_quic_options()), + filter_config_provider_manager_(filter_config_provider_manager) {} AccessLog::AccessLogManager& ListenerFactoryContextBaseImpl::accessLogManager() { return server_.accessLogManager(); @@ -307,7 +309,7 @@ ListenerFactoryContextBaseImpl::createDynamicFilterConfigProvider( } Configuration::DownstreamFilterConfigProviderManagerPtr ListenerFactoryContextBaseImpl::downstreamFilterConfigProviderManager() { - return nullptr; + return filter_config_provider_manager_; } Network::DrainDecision& ListenerFactoryContextBaseImpl::drainDecision() { return *this; } @@ -348,7 +350,8 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config, continue_on_listener_filters_timeout_(config.continue_on_listener_filters_timeout()), listener_factory_context_(std::make_shared( parent.server_, validation_visitor_, config, this, *this, - parent_.factory_->createDrainManager(config.drain_type()))), + parent_.factory_->createDrainManager(config.drain_type()), + parent_.filter_config_provider_manager_)), reuse_port_(getReusePortOrDefault(parent_.server_, config_, socket_type_)), cx_limit_runtime_key_("envoy.resource_limits.listener." + config_.name() + ".connection_limit"), diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.h b/source/extensions/listener_managers/listener_manager/listener_impl.h index 0d89f5d3ac25..f0b76136b919 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_impl.h @@ -19,6 +19,7 @@ #include "source/common/common/basic_resource_impl.h" #include "source/common/common/logger.h" #include "source/common/config/metadata.h" +#include "source/common/filter/config_discovery_impl.h" #include "source/common/init/manager_impl.h" #include "source/common/init/target_impl.h" #include "source/common/quic/quic_stat_names.h" @@ -125,10 +126,10 @@ class ListenSocketFactoryImpl : public Network::ListenSocketFactory, class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContext, public Network::DrainDecision { public: - ListenerFactoryContextBaseImpl(Envoy::Server::Instance& server, - ProtobufMessage::ValidationVisitor& validation_visitor, - const envoy::config::listener::v3::Listener& config, - Server::DrainManagerPtr drain_manager); + ListenerFactoryContextBaseImpl( + Envoy::Server::Instance& server, ProtobufMessage::ValidationVisitor& validation_visitor, + const envoy::config::listener::v3::Listener& config, Server::DrainManagerPtr drain_manager, + Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager); AccessLog::AccessLogManager& accessLogManager() override; Upstream::ClusterManager& clusterManager() override; Event::Dispatcher& mainThreadDispatcher() override; @@ -189,6 +190,7 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex ProtobufMessage::ValidationVisitor& validation_visitor_; const Server::DrainManagerPtr drain_manager_; bool is_quic_; + Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager_{}; }; class ListenerImpl; @@ -198,13 +200,15 @@ class ListenerImpl; // listener update or during the lifetime of listener? class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryContext { public: - PerListenerFactoryContextImpl(Envoy::Server::Instance& server, - ProtobufMessage::ValidationVisitor& validation_visitor, - const envoy::config::listener::v3::Listener& config_message, - const Network::ListenerConfig* listener_config, - ListenerImpl& listener_impl, DrainManagerPtr drain_manager) + PerListenerFactoryContextImpl( + Envoy::Server::Instance& server, ProtobufMessage::ValidationVisitor& validation_visitor, + const envoy::config::listener::v3::Listener& config_message, + const Network::ListenerConfig* listener_config, ListenerImpl& listener_impl, + DrainManagerPtr drain_manager, + Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager) : listener_factory_context_base_(std::make_shared( - server, validation_visitor, config_message, std::move(drain_manager))), + server, validation_visitor, config_message, std::move(drain_manager), + filter_config_provider_manager)), listener_config_(listener_config), listener_impl_(listener_impl) {} PerListenerFactoryContextImpl( std::shared_ptr listener_factory_context_base, diff --git a/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc b/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc index 66f4f777d5aa..ff4130595034 100644 --- a/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc @@ -350,6 +350,8 @@ ListenerManagerImpl::ListenerManagerImpl(Instance& server, bool enable_dispatcher_stats, Quic::QuicStatNames& quic_stat_names) : server_(server), factory_(std::move(factory)), + filter_config_provider_manager_( + std::make_shared()), scope_(server.stats().createScope("listener_manager.")), stats_(generateStats(*scope_)), enable_dispatcher_stats_(enable_dispatcher_stats), quic_stat_names_(quic_stat_names) { if (!factory_) { diff --git a/source/extensions/listener_managers/listener_manager/listener_manager_impl.h b/source/extensions/listener_managers/listener_manager/listener_manager_impl.h index 56bd78050836..f42a748fbc41 100644 --- a/source/extensions/listener_managers/listener_manager/listener_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_manager_impl.h @@ -236,6 +236,7 @@ class ListenerManagerImpl : public ListenerManager, Logger::Loggable factory_; + Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager_{}; private: using ListenerList = std::list; From 7eb780aa2f433fb5e31288b18cb182398a5c0003 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Mon, 2 Oct 2023 09:29:53 +0530 Subject: [PATCH 41/73] fix config test Signed-off-by: Rama Chavali --- test/mocks/server/factory_context.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index b9995c44f7a2..70dba2a2eabb 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -65,7 +65,7 @@ class MockFactoryContext : public virtual ListenerFactoryContext { } Configuration::DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() override { - return nullptr; + return filter_config_provider_manager_; } testing::NiceMock server_factory_context_; @@ -93,6 +93,8 @@ class MockFactoryContext : public virtual ListenerFactoryContext { Http::ContextImpl http_context_; Router::ContextImpl router_context_; testing::NiceMock api_; + Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager_{ + std::make_shared()}; }; class MockUpstreamFactoryContext : public UpstreamFactoryContext { From 0780f55f658a3b498ffe7f84109751099dc2ae13 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Mon, 2 Oct 2023 15:55:48 +0530 Subject: [PATCH 42/73] fix coverage Signed-off-by: Rama Chavali --- .../listener_manager/listener_manager_impl_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc b/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc index 628ce29c83b1..a8ab14e5190f 100644 --- a/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc +++ b/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc @@ -5792,6 +5792,9 @@ TEST_P(ListenerManagerImplWithRealFiltersTest, OriginalDstFilter) { EXPECT_EQ(&parent_context.timeSource(), &listener_factory_context->api().timeSource()); EXPECT_EQ(&parent_context.messageValidationContext(), &server_.messageValidationContext()); EXPECT_EQ(&parent_context.lifecycleNotifier(), &server_.lifecycleNotifier()); + envoy::config::core::v3::ExtensionConfigSource config_source; + EXPECT_EQ(&parent_context.createDynamicFilterConfigProvider(config_source, "", true, "", nullptr), + nullptr); Network::FilterChainFactory& filterChainFactory = listener.filterChainFactory(); Network::MockListenerFilterManager manager; From 2b294472406b35205356468690940f7bf1a3144a Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Mon, 2 Oct 2023 17:18:57 +0530 Subject: [PATCH 43/73] fix coverage Signed-off-by: Rama Chavali --- .../listener_manager/listener_manager_impl_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc b/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc index a8ab14e5190f..2e6cdc81ee12 100644 --- a/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc +++ b/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc @@ -5793,7 +5793,7 @@ TEST_P(ListenerManagerImplWithRealFiltersTest, OriginalDstFilter) { EXPECT_EQ(&parent_context.messageValidationContext(), &server_.messageValidationContext()); EXPECT_EQ(&parent_context.lifecycleNotifier(), &server_.lifecycleNotifier()); envoy::config::core::v3::ExtensionConfigSource config_source; - EXPECT_EQ(&parent_context.createDynamicFilterConfigProvider(config_source, "", true, "", nullptr), + EXPECT_EQ(parent_context.createDynamicFilterConfigProvider(config_source, "", true, "", nullptr), nullptr); Network::FilterChainFactory& filterChainFactory = listener.filterChainFactory(); From 07f24e156a88518a95469052e6f30d7adc6ab6cd Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 4 Oct 2023 17:27:46 +0530 Subject: [PATCH 44/73] add validation Signed-off-by: Rama Chavali --- .../filters/http/composite/action.cc | 6 +++ test/extensions/filters/http/composite/BUILD | 4 ++ .../filters/http/composite/filter_test.cc | 40 +++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index eb1fcbfec83b..d1e81adc98b2 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -14,6 +14,12 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( const auto& composite_action = MessageUtil::downcastAndValidate< const envoy::extensions::filters::http::composite::v3::ExecuteFilterAction&>( config, validation_visitor); + + if (composite_action.has_dynamic_config() && composite_action.has_typed_config()) { + throw EnvoyException( + fmt::format("Error: Only one of `dynamic_config` or `typed_config` can be set.")); + } + // Process dynamic filter configuration and setup extension configuration discovery service. if (composite_action.has_dynamic_config()) { auto config_discovery = composite_action.dynamic_config().config_discovery(); diff --git a/test/extensions/filters/http/composite/BUILD b/test/extensions/filters/http/composite/BUILD index f846deb7b778..bf7ee48d78f3 100644 --- a/test/extensions/filters/http/composite/BUILD +++ b/test/extensions/filters/http/composite/BUILD @@ -19,8 +19,12 @@ envoy_extension_cc_test( "//source/common/http:header_map_lib", "//source/extensions/filters/http/composite:config", "//source/extensions/filters/http/composite:filter_lib", + "//source/extensions/filters/http/fault:config", + "//source/extensions/filters/http/fault:fault_filter_lib", "//test/mocks/access_log:access_log_mocks", "//test/mocks/http:http_mocks", + "//test/mocks/server:factory_context_mocks", + "//test/mocks/server:instance_mocks", ], ) diff --git a/test/extensions/filters/http/composite/filter_test.cc b/test/extensions/filters/http/composite/filter_test.cc index 4085800097ac..acaf9814e286 100644 --- a/test/extensions/filters/http/composite/filter_test.cc +++ b/test/extensions/filters/http/composite/filter_test.cc @@ -2,10 +2,13 @@ #include "envoy/http/metadata_interface.h" +#include "source/extensions/filters/http/composite/action.h" #include "source/extensions/filters/http/composite/filter.h" #include "test/mocks/access_log/mocks.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/server/factory_context.h" +#include "test/mocks/server/instance.h" #include "gtest/gtest.h" @@ -234,6 +237,43 @@ TEST_F(FilterTest, StreamFilterDelegationMultipleAccessLoggers) { AccessLog::AccessLogType::NotSet); } +// Validate that when dynamic_config and typed_config are both set, an exception is thrown. +TEST(ConfigTest, TestConfig) { + const std::string yaml_string = R"EOF( + typed_config: + name: set-response-code + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.fault.v3.HTTPFault + abort: + http_status: 503 + percentage: + numerator: 0 + denominator: HUNDRED + dynamic_config: + name: set-response-code + config_discovery: + config_source: + resource_api_version: V3 + path_config_source: + path: "{{ test_tmpdir }}/set_response_code.yaml" + type_urls: + - type.googleapis.com/test.integration.filters.SetResponseCodeFilterConfig +)EOF"; + + envoy::extensions::filters::http::composite::v3::ExecuteFilterAction config; + TestUtility::loadFromYaml(yaml_string, config); + + testing::NiceMock server_factory_context; + testing::NiceMock factory_context; + Envoy::Http::Matching::HttpFilterActionContext action_context{"test", factory_context, + server_factory_context}; + ExecuteFilterActionFactory factory; + EXPECT_THROW_WITH_MESSAGE( + factory.createActionFactoryCb(config, action_context, + ProtobufMessage::getStrictValidationVisitor()), + EnvoyException, "Error: Only one of `dynamic_config` or `typed_config` can be set."); +} + } // namespace } // namespace Composite } // namespace HttpFilters From 59050d755afd4ff61e2458efef3f4ae5aa4f930b Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 17 Oct 2023 08:36:32 +0530 Subject: [PATCH 45/73] add docs Signed-off-by: Rama Chavali --- .../extensions/filters/http/composite/v3/composite.proto | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index 507175d08edb..b9382b12d3bf 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -48,11 +48,14 @@ message DynamicConfig { message ExecuteFilterAction { // Filter specific configuration which depends on the filter being // instantiated. See the supported filters for further documentation. + // Only one of ``typed_config`` or ``dynamic_config`` can be set. // [#extension-category: envoy.filters.listener,envoy.filters.udp_listener] config.core.v3.TypedExtensionConfig typed_config = 1 [(udpa.annotations.field_migrate).oneof_promotion = "config_type"]; // Dynamic configuration of filter obtained via extension configuration discovery // service. - DynamicConfig dynamic_config = 2; + // Only one of ``typed_config`` or ``dynamic_config`` can be set. + DynamicConfig dynamic_config = 2 + [(udpa.annotations.field_migrate).oneof_promotion = "config_type"]; } From cbdbab864c1dd12c28d6c2e4be5b215fb6d3ae94 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 17 Oct 2023 08:50:38 +0530 Subject: [PATCH 46/73] format Signed-off-by: Rama Chavali --- .../http/composite/composite_filter_integration_test.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index 11a31b5929b5..b8c345f45690 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -204,10 +204,11 @@ TEST_P(CompositeFilterIntegrationTest, TestBasic) { TEST_P(CompositeFilterIntegrationTest, TestBasicDynamicFilter) { prependCompositeDynamicFilter("composite-dynamic"); initialize(); - test_server_->waitForCounterGe("extension_config_discovery.http_filter.set-response-code.config_reload", 1); + test_server_->waitForCounterGe( + "extension_config_discovery.http_filter.set-response-code.config_reload", 1); test_server_->waitUntilListenersReady(); test_server_->waitForGaugeGe("listener_manager.workers_started", 1); - + codec_client_ = makeHttpConnection(lookupPort("http")); { From 8c5896fdc06193147a6fe8d20b9c4838cf8b1e9c Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 17 Oct 2023 09:18:30 +0530 Subject: [PATCH 47/73] format again Signed-off-by: Rama Chavali --- changelogs/current.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index ab945b96ef10..45b4788a78ae 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -400,7 +400,7 @@ new_features: - area: composite filter change: | added :ref:`ExtensionConfiguration discovery service` support for - :ref:`composite filter `. + :ref:`composite filter `. - area: router change: | Added ``metadata`` support for :ref:`virtual host ` and From 7515803cb6f6520e1c1ef1d217b11b0cd02e10d1 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 24 Oct 2023 12:40:27 +0530 Subject: [PATCH 48/73] fix extension category Signed-off-by: Rama Chavali --- api/envoy/extensions/filters/http/composite/v3/composite.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index b9382b12d3bf..21578b02a9aa 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -49,7 +49,7 @@ message ExecuteFilterAction { // Filter specific configuration which depends on the filter being // instantiated. See the supported filters for further documentation. // Only one of ``typed_config`` or ``dynamic_config`` can be set. - // [#extension-category: envoy.filters.listener,envoy.filters.udp_listener] + // [#extension-category: envoy.filters.http.composite] config.core.v3.TypedExtensionConfig typed_config = 1 [(udpa.annotations.field_migrate).oneof_promotion = "config_type"]; From 4253d390a10903354f7a77de46398aee296c4db5 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 1 Nov 2023 11:48:46 +0530 Subject: [PATCH 49/73] add missing config filter Signed-off-by: Rama Chavali --- source/common/http/filter_chain_helper.cc | 18 ----- source/common/http/filter_chain_helper.h | 20 ++++- .../filters/http/composite/action.cc | 43 +++++----- .../composite_filter_integration_test.cc | 79 ++++++++++++++++++- 4 files changed, 114 insertions(+), 46 deletions(-) diff --git a/source/common/http/filter_chain_helper.cc b/source/common/http/filter_chain_helper.cc index 1e9d1a8b1ce0..5278da2a3d88 100644 --- a/source/common/http/filter_chain_helper.cc +++ b/source/common/http/filter_chain_helper.cc @@ -5,32 +5,14 @@ #include "envoy/registry/registry.h" -#include "source/common/common/empty_string.h" #include "source/common/common/fmt.h" #include "source/common/config/utility.h" #include "source/common/http/utility.h" #include "source/common/protobuf/utility.h" -#include "source/extensions/filters/http/common/pass_through_filter.h" namespace Envoy { namespace Http { -// Allows graceful handling of missing configuration for ECDS. -class MissingConfigFilter : public Http::PassThroughDecoderFilter { -public: - Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap&, bool) override { - decoder_callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoFilterConfigFound); - decoder_callbacks_->sendLocalReply(Http::Code::InternalServerError, EMPTY_STRING, nullptr, - absl::nullopt, EMPTY_STRING); - return Http::FilterHeadersStatus::StopIteration; - } -}; - -static Http::FilterFactoryCb MissingConfigFilterFactory = - [](Http::FilterChainFactoryCallbacks& cb) { - cb.addStreamDecoderFilter(std::make_shared()); - }; - void FilterChainUtility::createFilterChainForFactories( Http::FilterChainManager& manager, const FilterChainOptions& options, const FilterFactoriesList& filter_factories) { diff --git a/source/common/http/filter_chain_helper.h b/source/common/http/filter_chain_helper.h index a9d20583fced..bf300adbf678 100644 --- a/source/common/http/filter_chain_helper.h +++ b/source/common/http/filter_chain_helper.h @@ -6,9 +6,11 @@ #include "envoy/filter/config_provider_manager.h" #include "envoy/http/filter.h" +#include "source/common/common/empty_string.h" #include "source/common/common/logger.h" #include "source/common/filter/config_discovery_impl.h" #include "source/common/http/dependency_manager.h" +#include "source/extensions/filters/http/common/pass_through_filter.h" namespace Envoy { namespace Http { @@ -17,9 +19,25 @@ using UpstreamFilterConfigProviderManager = Filter::FilterConfigProviderManager; +// Allows graceful handling of missing configuration for ECDS. +class MissingConfigFilter : public Http::PassThroughDecoderFilter { +public: + Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap&, bool) override { + decoder_callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoFilterConfigFound); + decoder_callbacks_->sendLocalReply(Http::Code::InternalServerError, EMPTY_STRING, nullptr, + absl::nullopt, EMPTY_STRING); + return Http::FilterHeadersStatus::StopIteration; + } +}; + +static Http::FilterFactoryCb MissingConfigFilterFactory = + [](Http::FilterChainFactoryCallbacks& cb) { + cb.addStreamDecoderFilter(std::make_shared()); + }; + class FilterChainUtility : Logger::Loggable { public: - struct FilterFactoryProvider { + struct FilterFactoryProvider { Filter::FilterConfigProviderPtr provider; // If true, this filter is disabled by default and must be explicitly enabled by // route configuration. diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 25c93c9d2e40..737a0150d855 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -29,10 +29,12 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( config_discovery, composite_action.dynamic_config().name(), false, "http", nullptr); return [provider = std::move(provider)]() -> Matcher::ActionPtr { auto config_value = provider->config(); - if (!config_value.has_value()) { - throw EnvoyException("Failed to get dynamic config for filter"); + if (config_value.has_value()) { + auto factory_cb = config_value.value().get().factory_cb; + return std::make_unique(factory_cb); } - auto factory_cb = config_value.value().get().factory_cb; + // There is no dynamic config available. Apply missing config filter. + auto factory_cb = Envoy::Http::MissingConfigFilterFactory; return std::make_unique(factory_cb); }; } else { @@ -42,29 +44,24 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( composite_action.typed_config()); ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig( composite_action.typed_config().typed_config(), validation_visitor, factory); - Envoy::Http::FilterFactoryCb callback = nullptr; + Envoy::Http::FilterFactoryCb callback = nullptr; - // First, try to create the filter factory creation function from factory context (if exists). - if (context.factory_context_.has_value()) { - callback = factory.createFilterFactoryFromProto(*message, context.stat_prefix_, - context.factory_context_.value()); - } + // First, try to create the filter factory creation function from factory context (if exists). + if (context.factory_context_.has_value()) { + callback = factory.createFilterFactoryFromProto(*message, context.stat_prefix_, + context.factory_context_.value()); + } - // If above failed, try to create the filter factory creation function from server factory - // context (if exists). - if (callback == nullptr && context.server_factory_context_.has_value()) { - callback = factory.createFilterFactoryFromProtoWithServerContext( - *message, context.stat_prefix_, context.server_factory_context_.value()); - } + // If above failed, try to create the filter factory creation function from server factory + // context (if exists). + if (callback == nullptr && context.server_factory_context_.has_value()) { + callback = factory.createFilterFactoryFromProtoWithServerContext( + *message, context.stat_prefix_, context.server_factory_context_.value()); + } - if (callback == nullptr) { - throwEnvoyExceptionOrPanic("Failed to get filter factory creation function"); - } - - return [cb = std::move(callback)]() -> Matcher::ActionPtr { - return std::make_unique(cb); - }; - + return [cb = std::move(callback)]() -> Matcher::ActionPtr { + return std::make_unique(cb); + }; } } diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index 67addc267bff..4c063c2d42e0 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -139,7 +139,8 @@ class CompositeFilterIntegrationTest : public testing::TestWithParamadd_clusters(); + ecds_cluster->MergeFrom(bootstrap.static_resources().clusters()[0]); + ecds_cluster->set_name("ecds_cluster"); + ecds_cluster->mutable_load_assignment()->set_cluster_name("ecds_cluster"); + }); + HttpIntegrationTest::initialize(); + } + + void createUpstreams() override { + BaseIntegrationTest::createUpstreams(); + addFakeUpstream(Http::CodecType::HTTP2); + } }; INSTANTIATE_TEST_SUITE_P(IpVersions, CompositeFilterIntegrationTest, @@ -247,6 +303,21 @@ TEST_P(CompositeFilterIntegrationTest, TestBasicDynamicFilter) { } } +// Verifies that if ECDS response is not sent, the missing filter config is applied that returns +// 500. +TEST_P(CompositeFilterIntegrationTest, TestMissingDynamicFilter) { + prependMissingCompositeDynamicFilter("composite-dynamic-missing"); + + initialize(); + test_server_->waitUntilListenersReady(); + test_server_->waitForGaugeGe("listener_manager.workers_started", 1); + + codec_client_ = makeHttpConnection(lookupPort("http")); + auto response = codec_client_->makeRequestWithBody(match_request_headers_, 1024); + ASSERT_TRUE(response->waitForEndStream()); + EXPECT_THAT(response->headers(), Http::HttpStatusIs("500")); +} + // Verifies function of the per-route config in the ExtensionWithMatcher class. TEST_P(CompositeFilterIntegrationTest, TestPerRoute) { prependCompositeFilter(); From 53ca6981d1a2a55c2fc846f6893748d6c7fe5f50 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 1 Nov 2023 12:09:05 +0530 Subject: [PATCH 50/73] fix proto Signed-off-by: Rama Chavali --- api/envoy/extensions/filters/http/composite/v3/composite.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index 21578b02a9aa..d4ec02001049 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -49,7 +49,7 @@ message ExecuteFilterAction { // Filter specific configuration which depends on the filter being // instantiated. See the supported filters for further documentation. // Only one of ``typed_config`` or ``dynamic_config`` can be set. - // [#extension-category: envoy.filters.http.composite] + // [#extension-category: envoy.filters.http] config.core.v3.TypedExtensionConfig typed_config = 1 [(udpa.annotations.field_migrate).oneof_promotion = "config_type"]; From 57a3e16f64a3a44a20869196a1f258f899ff551f Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 1 Nov 2023 13:06:24 +0530 Subject: [PATCH 51/73] add stat check Signed-off-by: Rama Chavali --- .../filters/http/composite/composite_filter_integration_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index 4c063c2d42e0..f97f0a236cc0 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -309,6 +309,8 @@ TEST_P(CompositeFilterIntegrationTest, TestMissingDynamicFilter) { prependMissingCompositeDynamicFilter("composite-dynamic-missing"); initialize(); + test_server_->waitForCounterGe( + "extension_config_discovery.http_filter.missing-config.config_fail", 1); test_server_->waitUntilListenersReady(); test_server_->waitForGaugeGe("listener_manager.workers_started", 1); From 85c23a88f3d0e2af7f161e681d6fb27e4d73a2ee Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 1 Nov 2023 15:51:40 +0530 Subject: [PATCH 52/73] move away from throw Exception Signed-off-by: Rama Chavali --- source/extensions/filters/http/composite/action.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 737a0150d855..d139c31cf31b 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -16,7 +16,7 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( config, validation_visitor); if (composite_action.has_dynamic_config() && composite_action.has_typed_config()) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("Error: Only one of `dynamic_config` or `typed_config` can be set.")); } From 02aa64ebe33e0a6b1ea05f0cfe91833f8bcd6d33 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 1 Nov 2023 16:14:24 +0530 Subject: [PATCH 53/73] fix compile Signed-off-by: Rama Chavali --- test/common/http/filter_chain_helper_test.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/common/http/filter_chain_helper_test.cc b/test/common/http/filter_chain_helper_test.cc index cba2ade9ed44..220bb8328d7b 100644 --- a/test/common/http/filter_chain_helper_test.cc +++ b/test/common/http/filter_chain_helper_test.cc @@ -60,9 +60,8 @@ TEST(FilterChainUtilityTest, CreateFilterChainForFactoriesWithRouteDisabledAndDe for (const auto& name : {"filter_0", "filter_1", "filter_2"}) { auto provider = - std::make_unique>( - Filter::NamedHttpFilterFactoryCb{"filter_type_name", - [](FilterChainFactoryCallbacks&) {}}, + std::make_unique>( + Http::NamedHttpFilterFactoryCb{"filter_type_name", [](FilterChainFactoryCallbacks&) {}}, name); filter_factories.push_back({std::move(provider), true}); } From c51441a61ab6a85c7be8c5375acc3136f30f0942 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 9 Nov 2023 14:06:24 +0530 Subject: [PATCH 54/73] address more review comments Signed-off-by: Rama Chavali --- envoy/server/factory_context.h | 16 +++++-------- .../filters/http/composite/action.cc | 4 ++-- .../filter_chain_manager_impl.cc | 10 ++++---- .../filter_chain_manager_impl.h | 8 +++---- .../listener_manager/listener_impl.cc | 24 ++++++++----------- .../listener_manager/listener_impl.h | 22 +++++++---------- .../listener_manager/listener_manager_impl.h | 2 +- source/server/factory_context_impl.cc | 8 +++---- source/server/factory_context_impl.h | 10 ++++---- .../listener_manager_impl_test.cc | 3 +-- test/mocks/server/factory_context.h | 7 +++--- .../server/filter_chain_factory_context.h | 5 ++-- test/mocks/server/listener_factory_context.h | 5 ++-- 13 files changed, 51 insertions(+), 73 deletions(-) diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index 9898f7cf05e9..abd0995c90e9 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -218,7 +218,7 @@ class FactoryContext; using DownstreamFilterConfigProviderManager = Filter::FilterConfigProviderManager; -using DownstreamFilterConfigProviderManagerPtr = +using DownstreamFilterConfigProviderManagerSharedPtr = std::shared_ptr; /** * Context passed to network and HTTP filters to access server resources. @@ -294,24 +294,20 @@ class FactoryContext : public virtual ListenerAccessLogFactoryContext { * @param filter_config_name the filter config resource name. * @param last_filter_in_filter_chain indicates whether this filter is the last filter in the * configured chain - * @param filter_chain_type is the filter chain type - * @param listener_filter_matcher is the filter matcher for TCP listener filter. nullptr for other - * filter types. * * @return HttpExtensionConfigProvider */ - virtual Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( + virtual Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) PURE; + const std::string& filter_config_name, bool last_filter_in_filter_chain) PURE; /** * Returns the downstream filter config provider manager. * - * @return DownstreamFilterConfigProviderManagerPtr + * @return DownstreamFilterConfigProviderManagerSharedPtr */ - virtual DownstreamFilterConfigProviderManagerPtr downstreamFilterConfigProviderManager() PURE; + virtual DownstreamFilterConfigProviderManagerSharedPtr + downstreamFilterConfigProviderManager() PURE; }; /** diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index d139c31cf31b..a2fc41824906 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -25,8 +25,8 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( auto config_discovery = composite_action.dynamic_config().config_discovery(); Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); // Create a dynamic filter config provider and register it with the server factory context. - auto provider = factory_context.createDynamicFilterConfigProvider( - config_discovery, composite_action.dynamic_config().name(), false, "http", nullptr); + auto provider = factory_context.createHttpDynamicFilterConfigProvider( + config_discovery, composite_action.dynamic_config().name(), false); return [provider = std::move(provider)]() -> Matcher::ActionPtr { auto config_value = provider->config(); if (config_value.has_value()) { diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc index 40d23906a896..941f0db06fe5 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc @@ -195,17 +195,15 @@ bool PerFilterChainFactoryContextImpl::isQuicListener() const { } Configuration::HttpExtensionConfigProvider -PerFilterChainFactoryContextImpl::createDynamicFilterConfigProvider( +PerFilterChainFactoryContextImpl::createHttpDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) { + const std::string& filter_config_name, bool last_filter_in_filter_chain) { return downstreamFilterConfigProviderManager()->createDynamicFilterConfigProvider( config_source, filter_config_name, getServerFactoryContext(), *this, clusterManager(), - last_filter_in_filter_chain, filter_chain_type, listener_filter_matcher); + last_filter_in_filter_chain, "http", nullptr); } -Configuration::DownstreamFilterConfigProviderManagerPtr +Configuration::DownstreamFilterConfigProviderManagerSharedPtr PerFilterChainFactoryContextImpl::downstreamFilterConfigProviderManager() { return parent_context_.downstreamFilterConfigProviderManager(); } diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h index 3c4d017fc717..b2efb21b6838 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h @@ -90,12 +90,10 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; - Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( + Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - Configuration::DownstreamFilterConfigProviderManagerPtr + const std::string& filter_config_name, bool last_filter_in_filter_chain) override; + Configuration::DownstreamFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override; void startDraining() override { is_draining_.store(true); } diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.cc b/source/extensions/listener_managers/listener_manager/listener_impl.cc index 574627aebe05..880106ac4abf 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_impl.cc @@ -230,7 +230,7 @@ std::string listenerStatsScope(const envoy::config::listener::v3::Listener& conf ListenerFactoryContextBaseImpl::ListenerFactoryContextBaseImpl( Envoy::Server::Instance& server, ProtobufMessage::ValidationVisitor& validation_visitor, const envoy::config::listener::v3::Listener& config, DrainManagerPtr drain_manager, - Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager) + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager) : server_(server), metadata_(config.metadata()), typed_metadata_(config.metadata()), direction_(config.traffic_direction()), global_scope_(server.stats().createScope("")), listener_scope_( @@ -302,12 +302,11 @@ ListenerFactoryContextBaseImpl::getTransportSocketFactoryContext() const { Stats::Scope& ListenerFactoryContextBaseImpl::listenerScope() { return *listener_scope_; } bool ListenerFactoryContextBaseImpl::isQuicListener() const { return is_quic_; } Configuration::HttpExtensionConfigProvider -ListenerFactoryContextBaseImpl::createDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool, - const std::string&, const Network::ListenerFilterMatcherSharedPtr&) { +ListenerFactoryContextBaseImpl::createHttpDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool) { return nullptr; } -Configuration::DownstreamFilterConfigProviderManagerPtr +Configuration::DownstreamFilterConfigProviderManagerSharedPtr ListenerFactoryContextBaseImpl::downstreamFilterConfigProviderManager() { return filter_config_provider_manager_; } @@ -967,16 +966,13 @@ bool PerListenerFactoryContextImpl::isQuicListener() const { return listener_factory_context_base_->isQuicListener(); } Configuration::HttpExtensionConfigProvider -PerListenerFactoryContextImpl::createDynamicFilterConfigProvider( +PerListenerFactoryContextImpl::createHttpDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) { - return listener_factory_context_base_->createDynamicFilterConfigProvider( - config_source, filter_config_name, last_filter_in_filter_chain, filter_chain_type, - listener_filter_matcher); -} -Configuration::DownstreamFilterConfigProviderManagerPtr + const std::string& filter_config_name, bool last_filter_in_filter_chain) { + return listener_factory_context_base_->createHttpDynamicFilterConfigProvider( + config_source, filter_config_name, last_filter_in_filter_chain); +} +Configuration::DownstreamFilterConfigProviderManagerSharedPtr PerListenerFactoryContextImpl::downstreamFilterConfigProviderManager() { return listener_factory_context_base_->downstreamFilterConfigProviderManager(); } diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.h b/source/extensions/listener_managers/listener_manager/listener_impl.h index f0b76136b919..584ab6dc4ed3 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_impl.h @@ -129,7 +129,7 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex ListenerFactoryContextBaseImpl( Envoy::Server::Instance& server, ProtobufMessage::ValidationVisitor& validation_visitor, const envoy::config::listener::v3::Listener& config, Server::DrainManagerPtr drain_manager, - Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager); + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager); AccessLog::AccessLogManager& accessLogManager() override; Upstream::ClusterManager& clusterManager() override; Event::Dispatcher& mainThreadDispatcher() override; @@ -161,12 +161,10 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; - Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( + Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - Configuration::DownstreamFilterConfigProviderManagerPtr + const std::string& filter_config_name, bool last_filter_in_filter_chain) override; + Configuration::DownstreamFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override; // DrainDecision @@ -190,7 +188,7 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex ProtobufMessage::ValidationVisitor& validation_visitor_; const Server::DrainManagerPtr drain_manager_; bool is_quic_; - Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager_{}; + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{}; }; class ListenerImpl; @@ -205,7 +203,7 @@ class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryConte const envoy::config::listener::v3::Listener& config_message, const Network::ListenerConfig* listener_config, ListenerImpl& listener_impl, DrainManagerPtr drain_manager, - Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager) + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager) : listener_factory_context_base_(std::make_shared( server, validation_visitor, config_message, std::move(drain_manager), filter_config_provider_manager)), @@ -249,12 +247,10 @@ class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryConte Stats::Scope& listenerScope() override; bool isQuicListener() const override; - Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( + Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - Configuration::DownstreamFilterConfigProviderManagerPtr + const std::string& filter_config_name, bool last_filter_in_filter_chain) override; + Configuration::DownstreamFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override; // ListenerFactoryContext diff --git a/source/extensions/listener_managers/listener_manager/listener_manager_impl.h b/source/extensions/listener_managers/listener_manager/listener_manager_impl.h index f42a748fbc41..de7aae308307 100644 --- a/source/extensions/listener_managers/listener_manager/listener_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_manager_impl.h @@ -236,7 +236,7 @@ class ListenerManagerImpl : public ListenerManager, Logger::Loggable factory_; - Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager_{}; + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{}; private: using ListenerList = std::list; diff --git a/source/server/factory_context_impl.cc b/source/server/factory_context_impl.cc index 77e17e906fe2..797be4c239b6 100644 --- a/source/server/factory_context_impl.cc +++ b/source/server/factory_context_impl.cc @@ -63,12 +63,12 @@ envoy::config::core::v3::TrafficDirection FactoryContextImpl::direction() const Network::DrainDecision& FactoryContextImpl::drainDecision() { return drain_decision_; } Stats::Scope& FactoryContextImpl::listenerScope() { return listener_scope_; } bool FactoryContextImpl::isQuicListener() const { return is_quic_; } -Configuration::HttpExtensionConfigProvider FactoryContextImpl::createDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool, - const std::string&, const Network::ListenerFilterMatcherSharedPtr&) { +Configuration::HttpExtensionConfigProvider +FactoryContextImpl::createHttpDynamicFilterConfigProvider( + const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool) { return nullptr; } -Configuration::DownstreamFilterConfigProviderManagerPtr +Configuration::DownstreamFilterConfigProviderManagerSharedPtr FactoryContextImpl::downstreamFilterConfigProviderManager() { return filter_config_provider_manager_; } diff --git a/source/server/factory_context_impl.h b/source/server/factory_context_impl.h index 58abc9973059..86befe86463f 100644 --- a/source/server/factory_context_impl.h +++ b/source/server/factory_context_impl.h @@ -49,12 +49,10 @@ class FactoryContextImpl : public Configuration::FactoryContext { Network::DrainDecision& drainDecision() override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; - Configuration::HttpExtensionConfigProvider createDynamicFilterConfigProvider( + Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain, - const std::string& filter_chain_type, - const Network::ListenerFilterMatcherSharedPtr& listener_filter_matcher) override; - Configuration::DownstreamFilterConfigProviderManagerPtr + const std::string& filter_config_name, bool last_filter_in_filter_chain) override; + Configuration::DownstreamFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override; private: @@ -64,7 +62,7 @@ class FactoryContextImpl : public Configuration::FactoryContext { Stats::Scope& global_scope_; Stats::Scope& listener_scope_; bool is_quic_; - Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager_{}; + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{}; }; } // namespace Server diff --git a/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc b/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc index b66ffbc761c9..dba7de2ec458 100644 --- a/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc +++ b/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc @@ -5793,8 +5793,7 @@ TEST_P(ListenerManagerImplWithRealFiltersTest, OriginalDstFilter) { EXPECT_EQ(&parent_context.messageValidationContext(), &server_.messageValidationContext()); EXPECT_EQ(&parent_context.lifecycleNotifier(), &server_.lifecycleNotifier()); envoy::config::core::v3::ExtensionConfigSource config_source; - EXPECT_EQ(parent_context.createDynamicFilterConfigProvider(config_source, "", true, "", nullptr), - nullptr); + EXPECT_EQ(parent_context.createDynamicFilterConfigProvider(config_source, "", true), nullptr); Network::FilterChainFactory& filterChainFactory = listener.filterChainFactory(); Network::MockListenerFilterManager manager; diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index 70dba2a2eabb..dee378ac7552 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -59,11 +59,10 @@ class MockFactoryContext : public virtual ListenerFactoryContext { HttpExtensionConfigProvider createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, bool, const std::string&, - const Network::ListenerFilterMatcherSharedPtr&) override { + const std::string&, bool) override { return nullptr; } - Configuration::DownstreamFilterConfigProviderManagerPtr + Configuration::DownstreamFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override { return filter_config_provider_manager_; } @@ -93,7 +92,7 @@ class MockFactoryContext : public virtual ListenerFactoryContext { Http::ContextImpl http_context_; Router::ContextImpl router_context_; testing::NiceMock api_; - Configuration::DownstreamFilterConfigProviderManagerPtr filter_config_provider_manager_{ + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ std::make_shared()}; }; diff --git a/test/mocks/server/filter_chain_factory_context.h b/test/mocks/server/filter_chain_factory_context.h index b5ab43a657ec..327c29f7c575 100644 --- a/test/mocks/server/filter_chain_factory_context.h +++ b/test/mocks/server/filter_chain_factory_context.h @@ -14,11 +14,10 @@ class MockFilterChainFactoryContext : public MockFactoryContext, public FilterCh HttpExtensionConfigProvider createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, bool, const std::string&, - const Network::ListenerFilterMatcherSharedPtr&) override { + const std::string&, bool) override { return nullptr; } - Configuration::DownstreamFilterConfigProviderManagerPtr + Configuration::DownstreamFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override { return nullptr; } diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index 3a025b6030cb..69a7f3d7ef77 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -58,12 +58,11 @@ class MockListenerFactoryContext : public ListenerFactoryContext { HttpExtensionConfigProvider createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, bool, const std::string&, - const Network::ListenerFilterMatcherSharedPtr&) override { + const std::string&, bool) override { return nullptr; } - Configuration::DownstreamFilterConfigProviderManagerPtr + Configuration::DownstreamFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override { return nullptr; } From 0ad934435b264f54752a5af0700414cacddef85c Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 9 Nov 2023 14:33:39 +0530 Subject: [PATCH 55/73] fix test Signed-off-by: Rama Chavali --- test/mocks/server/factory_context.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index dee378ac7552..c4654e1fc50f 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -58,8 +58,8 @@ class MockFactoryContext : public virtual ListenerFactoryContext { MOCK_METHOD(Api::Api&, api, ()); HttpExtensionConfigProvider - createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, bool) override { + createHttpDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, + const std::string&, bool) override { return nullptr; } Configuration::DownstreamFilterConfigProviderManagerSharedPtr From d48ce3551a2113ff0f987e023273574d9031485c Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 9 Nov 2023 15:12:18 +0530 Subject: [PATCH 56/73] fix test Signed-off-by: Rama Chavali --- .../listener_manager/listener_manager_impl_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc b/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc index dba7de2ec458..96397f69547f 100644 --- a/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc +++ b/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc @@ -5793,7 +5793,7 @@ TEST_P(ListenerManagerImplWithRealFiltersTest, OriginalDstFilter) { EXPECT_EQ(&parent_context.messageValidationContext(), &server_.messageValidationContext()); EXPECT_EQ(&parent_context.lifecycleNotifier(), &server_.lifecycleNotifier()); envoy::config::core::v3::ExtensionConfigSource config_source; - EXPECT_EQ(parent_context.createDynamicFilterConfigProvider(config_source, "", true), nullptr); + EXPECT_EQ(parent_context.createHttpDynamicFilterConfigProvider(config_source, "", true), nullptr); Network::FilterChainFactory& filterChainFactory = listener.filterChainFactory(); Network::MockListenerFilterManager manager; From 54c2a3ef62baae15913f35deda4f1cd97e8a94a6 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 9 Nov 2023 15:37:30 +0530 Subject: [PATCH 57/73] fix test Signed-off-by: Rama Chavali --- test/mocks/server/listener_factory_context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index 69a7f3d7ef77..032e1835c18c 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -57,7 +57,7 @@ class MockListenerFactoryContext : public ListenerFactoryContext { MOCK_METHOD(Api::Api&, api, ()); HttpExtensionConfigProvider - createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, + createHttpDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool) override { return nullptr; } From c2513b2ca5fc3d305aaf821a052e64fd10cb8769 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 9 Nov 2023 15:51:46 +0530 Subject: [PATCH 58/73] format Signed-off-by: Rama Chavali --- test/mocks/server/listener_factory_context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index 032e1835c18c..9486b4599be6 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -58,7 +58,7 @@ class MockListenerFactoryContext : public ListenerFactoryContext { HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, bool) override { + const std::string&, bool) override { return nullptr; } From c9477659cb7a324e89e843642138c4e8be4a0acd Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sat, 18 Nov 2023 12:44:15 +0530 Subject: [PATCH 59/73] move to server factory Signed-off-by: Rama Chavali --- envoy/server/factory_context.h | 39 +++++++++++++++---- .../filters/http/composite/action.cc | 3 +- .../network/http_connection_manager/config.cc | 2 +- source/server/server.h | 16 +++++++- test/mocks/server/server_factory_context.h | 22 +++++++++++ 5 files changed, 72 insertions(+), 10 deletions(-) diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index 1b470f117c38..4d25fec606c7 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -151,6 +151,14 @@ class CommonFactoryContext : public FactoryContextBase { virtual Init::Manager& initManager() PURE; }; +class FactoryContext; + +using DownstreamFilterConfigProviderManager = + Filter::FilterConfigProviderManager; +using DownstreamFilterConfigProviderManagerSharedPtr = + std::shared_ptr; + /** * ServerFactoryContext is an specialization of common interface for downstream and upstream network * filters. The implementation guarantees the lifetime is no shorter than server. It could be used @@ -184,6 +192,30 @@ class ServerFactoryContext : public virtual CommonFactoryContext { * @return envoy::config::bootstrap::v3::Bootstrap& the servers bootstrap configuration. */ virtual envoy::config::bootstrap::v3::Bootstrap& bootstrap() PURE; + + /** + * Create an FilterConfigProviderPtr for a filter config. The config providers may share + * the underlying subscriptions to the filter config discovery service. + * @param factory_context supplies the factory context. + * @param config_source supplies the extension configuration source for the filter configs. + * @param filter_config_name the filter config resource name. + * @param last_filter_in_filter_chain indicates whether this filter is the last filter in the + * configured chain + * + * @return HttpExtensionConfigProvider + */ + virtual Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( + Configuration::FactoryContext& factory_context, + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, bool last_filter_in_filter_chain) PURE; + + /** + * Returns the downstream filter config provider manager. + * + * @return DownstreamFilterConfigProviderManagerSharedPtr + */ + virtual DownstreamFilterConfigProviderManagerSharedPtr + downstreamFilterConfigProviderManager() PURE; }; /** @@ -213,13 +245,6 @@ class ListenerAccessLogFactoryContext : public virtual CommonFactoryContext { virtual ProcessContextOptRef processContext() PURE; }; -class FactoryContext; - -using DownstreamFilterConfigProviderManager = - Filter::FilterConfigProviderManager; -using DownstreamFilterConfigProviderManagerSharedPtr = - std::shared_ptr; /** * Context passed to network and HTTP filters to access server resources. * TODO(mattklein123): When we lock down visibility of the rest of the code, filters should only diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 3d0072d5ced5..9db3859bfece 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -24,8 +24,9 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( if (composite_action.has_dynamic_config()) { auto config_discovery = composite_action.dynamic_config().config_discovery(); Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); + Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); // Create a dynamic filter config provider and register it with the server factory context. - auto provider = factory_context.createHttpDynamicFilterConfigProvider( + auto provider = server_factory_context.createHttpDynamicFilterConfigProvider(factory_context, config_discovery, composite_action.dynamic_config().name(), false); return [provider = std::move(provider)]() -> Matcher::ActionPtr { auto config_value = provider->config(); diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 684c9312aef9..0b87cfe12265 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -230,7 +230,7 @@ Utility::Singletons Utility::createSingletons(Server::Configuration::FactoryCont auto tracer_manager = Tracing::TracerManagerImpl::singleton(context); std::shared_ptr - filter_config_provider_manager = context.downstreamFilterConfigProviderManager(); + filter_config_provider_manager = context.getServerFactoryContext().downstreamFilterConfigProviderManager(); return {date_provider, route_config_provider_manager, scoped_routes_config_provider_manager, tracer_manager, filter_config_provider_manager}; diff --git a/source/server/server.h b/source/server/server.h index 57c91c79711b..8ecfa2ccc549 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -172,7 +172,8 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, public Configuration::TransportSocketFactoryContext { public: explicit ServerFactoryContextImpl(Instance& server) - : server_(server), server_scope_(server_.stats().createScope("")) {} + : server_(server), server_scope_(server_.stats().createScope("")) , filter_config_provider_manager_( + std::make_shared()){} // Configuration::ServerFactoryContext Upstream::ClusterManager& clusterManager() override { return server_.clusterManager(); } @@ -197,6 +198,18 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, ServerLifecycleNotifier& lifecycleNotifier() override { return server_.lifecycleNotifier(); } Configuration::StatsConfig& statsConfig() override { return server_.statsConfig(); } envoy::config::bootstrap::v3::Bootstrap& bootstrap() override { return server_.bootstrap(); } + Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( + Configuration::FactoryContext& factory_context, + const envoy::config::core::v3::ExtensionConfigSource& config_source, + const std::string& filter_config_name, bool last_filter_in_filter_chain) override{ + return downstreamFilterConfigProviderManager()->createDynamicFilterConfigProvider( + config_source, filter_config_name, *this, factory_context, clusterManager(), last_filter_in_filter_chain, "http", nullptr); + } + + Configuration::DownstreamFilterConfigProviderManagerSharedPtr + downstreamFilterConfigProviderManager() override { + return filter_config_provider_manager_; + } // Configuration::TransportSocketFactoryContext ServerFactoryContext& serverFactoryContext() override { return *this; } @@ -218,6 +231,7 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, private: Instance& server_; Stats::ScopeSharedPtr server_scope_; + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{}; }; /** diff --git a/test/mocks/server/server_factory_context.h b/test/mocks/server/server_factory_context.h index d7bd11f4719f..9b9929a553f8 100644 --- a/test/mocks/server/server_factory_context.h +++ b/test/mocks/server/server_factory_context.h @@ -79,6 +79,15 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { MOCK_METHOD(ServerLifecycleNotifier&, lifecycleNotifier, ()); MOCK_METHOD(StatsConfig&, statsConfig, (), ()); MOCK_METHOD(AccessLog::AccessLogManager&, accessLogManager, (), ()); + HttpExtensionConfigProvider + createHttpDynamicFilterConfigProvider( Configuration::FactoryContext&, const envoy::config::core::v3::ExtensionConfigSource&, + const std::string&, bool) override { + return nullptr; + } + Configuration::DownstreamFilterConfigProviderManagerSharedPtr + downstreamFilterConfigProviderManager() override { + return filter_config_provider_manager_; + } testing::NiceMock cluster_manager_; testing::NiceMock dispatcher_; @@ -101,6 +110,8 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { Router::ContextImpl router_context_; envoy::config::bootstrap::v3::Bootstrap bootstrap_; testing::NiceMock options_; + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ + std::make_shared()}; }; // Stateless mock ServerFactoryContext for cases where it needs to be used concurrently in different @@ -134,6 +145,17 @@ class StatelessMockServerFactoryContext : public virtual ServerFactoryContext { MOCK_METHOD(ServerLifecycleNotifier&, lifecycleNotifier, ()); MOCK_METHOD(StatsConfig&, statsConfig, (), ()); MOCK_METHOD(AccessLog::AccessLogManager&, accessLogManager, (), ()); + HttpExtensionConfigProvider + createHttpDynamicFilterConfigProvider( Configuration::FactoryContext&, const envoy::config::core::v3::ExtensionConfigSource&, + const std::string&, bool) override { + return nullptr; + } + Configuration::DownstreamFilterConfigProviderManagerSharedPtr + downstreamFilterConfigProviderManager() override { + return filter_config_provider_manager_; + } + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ + std::make_shared()}; }; } // namespace Configuration From 7fe5c87b9f0ecef4b2614bd11cc929385a3767e2 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sat, 18 Nov 2023 13:44:49 +0530 Subject: [PATCH 60/73] remove from factory context Signed-off-by: Rama Chavali --- envoy/server/factory_context.h | 26 ++------------- .../filters/http/composite/action.cc | 13 ++++---- .../network/http_connection_manager/config.cc | 3 +- .../filter_chain_manager_impl.cc | 14 -------- .../filter_chain_manager_impl.h | 5 --- .../listener_manager/listener_impl.cc | 29 ++-------------- .../listener_manager/listener_impl.h | 33 ++++++------------- source/server/factory_context_impl.cc | 9 ----- source/server/factory_context_impl.h | 5 --- source/server/server.h | 16 +++++---- .../listener_manager_impl_test.cc | 2 -- test/mocks/server/factory_context.h | 12 ------- test/mocks/server/listener_factory_context.h | 11 ------- test/mocks/server/server_factory_context.h | 14 ++++---- 14 files changed, 41 insertions(+), 151 deletions(-) diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index 4d25fec606c7..325954a8dd9d 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -193,7 +193,7 @@ class ServerFactoryContext : public virtual CommonFactoryContext { */ virtual envoy::config::bootstrap::v3::Bootstrap& bootstrap() PURE; - /** + /** * Create an FilterConfigProviderPtr for a filter config. The config providers may share * the underlying subscriptions to the filter config discovery service. * @param factory_context supplies the factory context. @@ -205,7 +205,7 @@ class ServerFactoryContext : public virtual CommonFactoryContext { * @return HttpExtensionConfigProvider */ virtual Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( - Configuration::FactoryContext& factory_context, + Configuration::FactoryContext& factory_context, const envoy::config::core::v3::ExtensionConfigSource& config_source, const std::string& filter_config_name, bool last_filter_in_filter_chain) PURE; @@ -311,28 +311,6 @@ class FactoryContext : public virtual ListenerAccessLogFactoryContext { * @return Router::Context& a reference to the router context. */ virtual Router::Context& routerContext() PURE; - - /** - * Create an FilterConfigProviderPtr for a filter config. The config providers may share - * the underlying subscriptions to the filter config discovery service. - * @param config_source supplies the extension configuration source for the filter configs. - * @param filter_config_name the filter config resource name. - * @param last_filter_in_filter_chain indicates whether this filter is the last filter in the - * configured chain - * - * @return HttpExtensionConfigProvider - */ - virtual Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain) PURE; - - /** - * Returns the downstream filter config provider manager. - * - * @return DownstreamFilterConfigProviderManagerSharedPtr - */ - virtual DownstreamFilterConfigProviderManagerSharedPtr - downstreamFilterConfigProviderManager() PURE; }; /** diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 9db3859bfece..3e3db90450d9 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -24,10 +24,11 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( if (composite_action.has_dynamic_config()) { auto config_discovery = composite_action.dynamic_config().config_discovery(); Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); - Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); + Server::Configuration::ServerFactoryContext& server_factory_context = + context.server_factory_context_.value(); // Create a dynamic filter config provider and register it with the server factory context. - auto provider = server_factory_context.createHttpDynamicFilterConfigProvider(factory_context, - config_discovery, composite_action.dynamic_config().name(), false); + auto provider = server_factory_context.createHttpDynamicFilterConfigProvider( + factory_context, config_discovery, composite_action.dynamic_config().name(), false); return [provider = std::move(provider)]() -> Matcher::ActionPtr { auto config_value = provider->config(); if (config_value.has_value()) { @@ -50,9 +51,9 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( // First, try to create the filter factory creation function from factory context (if exists). if (context.factory_context_.has_value()) { auto callback_or_status = factory.createFilterFactoryFromProto( - *message, context.stat_prefix_, context.factory_context_.value()); - THROW_IF_STATUS_NOT_OK(callback_or_status, throw); - callback = callback_or_status.value(); + *message, context.stat_prefix_, context.factory_context_.value()); + THROW_IF_STATUS_NOT_OK(callback_or_status, throw); + callback = callback_or_status.value(); } // If above failed, try to create the filter factory creation function from server factory diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 0b87cfe12265..165b69309773 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -230,7 +230,8 @@ Utility::Singletons Utility::createSingletons(Server::Configuration::FactoryCont auto tracer_manager = Tracing::TracerManagerImpl::singleton(context); std::shared_ptr - filter_config_provider_manager = context.getServerFactoryContext().downstreamFilterConfigProviderManager(); + filter_config_provider_manager = + context.getServerFactoryContext().downstreamFilterConfigProviderManager(); return {date_provider, route_config_provider_manager, scoped_routes_config_provider_manager, tracer_manager, filter_config_provider_manager}; diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc index 941f0db06fe5..ec21386f1afb 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc @@ -194,20 +194,6 @@ bool PerFilterChainFactoryContextImpl::isQuicListener() const { return parent_context_.isQuicListener(); } -Configuration::HttpExtensionConfigProvider -PerFilterChainFactoryContextImpl::createHttpDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain) { - return downstreamFilterConfigProviderManager()->createDynamicFilterConfigProvider( - config_source, filter_config_name, getServerFactoryContext(), *this, clusterManager(), - last_filter_in_filter_chain, "http", nullptr); -} - -Configuration::DownstreamFilterConfigProviderManagerSharedPtr -PerFilterChainFactoryContextImpl::downstreamFilterConfigProviderManager() { - return parent_context_.downstreamFilterConfigProviderManager(); -} - FilterChainManagerImpl::FilterChainManagerImpl( const std::vector& addresses, Configuration::FactoryContext& factory_context, Init::Manager& init_manager, diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h index b2efb21b6838..565769438dff 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h @@ -90,11 +90,6 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; - Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain) override; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr - downstreamFilterConfigProviderManager() override; void startDraining() override { is_draining_.store(true); } private: diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.cc b/source/extensions/listener_managers/listener_manager/listener_impl.cc index 880106ac4abf..d0606d05b286 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_impl.cc @@ -229,15 +229,13 @@ std::string listenerStatsScope(const envoy::config::listener::v3::Listener& conf ListenerFactoryContextBaseImpl::ListenerFactoryContextBaseImpl( Envoy::Server::Instance& server, ProtobufMessage::ValidationVisitor& validation_visitor, - const envoy::config::listener::v3::Listener& config, DrainManagerPtr drain_manager, - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager) + const envoy::config::listener::v3::Listener& config, DrainManagerPtr drain_manager) : server_(server), metadata_(config.metadata()), typed_metadata_(config.metadata()), direction_(config.traffic_direction()), global_scope_(server.stats().createScope("")), listener_scope_( server_.stats().createScope(fmt::format("listener.{}.", listenerStatsScope(config)))), validation_visitor_(validation_visitor), drain_manager_(std::move(drain_manager)), - is_quic_(config.udp_listener_config().has_quic_options()), - filter_config_provider_manager_(filter_config_provider_manager) {} + is_quic_(config.udp_listener_config().has_quic_options()) {} AccessLog::AccessLogManager& ListenerFactoryContextBaseImpl::accessLogManager() { return server_.accessLogManager(); @@ -301,15 +299,6 @@ ListenerFactoryContextBaseImpl::getTransportSocketFactoryContext() const { } Stats::Scope& ListenerFactoryContextBaseImpl::listenerScope() { return *listener_scope_; } bool ListenerFactoryContextBaseImpl::isQuicListener() const { return is_quic_; } -Configuration::HttpExtensionConfigProvider -ListenerFactoryContextBaseImpl::createHttpDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool) { - return nullptr; -} -Configuration::DownstreamFilterConfigProviderManagerSharedPtr -ListenerFactoryContextBaseImpl::downstreamFilterConfigProviderManager() { - return filter_config_provider_manager_; -} Network::DrainDecision& ListenerFactoryContextBaseImpl::drainDecision() { return *this; } Server::DrainManager& ListenerFactoryContextBaseImpl::drainManager() { return *drain_manager_; } @@ -349,8 +338,7 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config, continue_on_listener_filters_timeout_(config.continue_on_listener_filters_timeout()), listener_factory_context_(std::make_shared( parent.server_, validation_visitor_, config, this, *this, - parent_.factory_->createDrainManager(config.drain_type()), - parent_.filter_config_provider_manager_)), + parent_.factory_->createDrainManager(config.drain_type()))), reuse_port_(getReusePortOrDefault(parent_.server_, config_, socket_type_)), cx_limit_runtime_key_("envoy.resource_limits.listener." + config_.name() + ".connection_limit"), @@ -965,17 +953,6 @@ Stats::Scope& PerListenerFactoryContextImpl::listenerScope() { bool PerListenerFactoryContextImpl::isQuicListener() const { return listener_factory_context_base_->isQuicListener(); } -Configuration::HttpExtensionConfigProvider -PerListenerFactoryContextImpl::createHttpDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain) { - return listener_factory_context_base_->createHttpDynamicFilterConfigProvider( - config_source, filter_config_name, last_filter_in_filter_chain); -} -Configuration::DownstreamFilterConfigProviderManagerSharedPtr -PerListenerFactoryContextImpl::downstreamFilterConfigProviderManager() { - return listener_factory_context_base_->downstreamFilterConfigProviderManager(); -} Init::Manager& PerListenerFactoryContextImpl::initManager() { return listener_impl_.initManager(); } diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.h b/source/extensions/listener_managers/listener_manager/listener_impl.h index 584ab6dc4ed3..a92e04a415d6 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_impl.h @@ -126,10 +126,10 @@ class ListenSocketFactoryImpl : public Network::ListenSocketFactory, class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContext, public Network::DrainDecision { public: - ListenerFactoryContextBaseImpl( - Envoy::Server::Instance& server, ProtobufMessage::ValidationVisitor& validation_visitor, - const envoy::config::listener::v3::Listener& config, Server::DrainManagerPtr drain_manager, - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager); + ListenerFactoryContextBaseImpl(Envoy::Server::Instance& server, + ProtobufMessage::ValidationVisitor& validation_visitor, + const envoy::config::listener::v3::Listener& config, + Server::DrainManagerPtr drain_manager); AccessLog::AccessLogManager& accessLogManager() override; Upstream::ClusterManager& clusterManager() override; Event::Dispatcher& mainThreadDispatcher() override; @@ -161,11 +161,6 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; - Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain) override; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr - downstreamFilterConfigProviderManager() override; // DrainDecision bool drainClose() const override { @@ -188,7 +183,6 @@ class ListenerFactoryContextBaseImpl final : public Configuration::FactoryContex ProtobufMessage::ValidationVisitor& validation_visitor_; const Server::DrainManagerPtr drain_manager_; bool is_quic_; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{}; }; class ListenerImpl; @@ -198,15 +192,13 @@ class ListenerImpl; // listener update or during the lifetime of listener? class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryContext { public: - PerListenerFactoryContextImpl( - Envoy::Server::Instance& server, ProtobufMessage::ValidationVisitor& validation_visitor, - const envoy::config::listener::v3::Listener& config_message, - const Network::ListenerConfig* listener_config, ListenerImpl& listener_impl, - DrainManagerPtr drain_manager, - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager) + PerListenerFactoryContextImpl(Envoy::Server::Instance& server, + ProtobufMessage::ValidationVisitor& validation_visitor, + const envoy::config::listener::v3::Listener& config_message, + const Network::ListenerConfig* listener_config, + ListenerImpl& listener_impl, DrainManagerPtr drain_manager) : listener_factory_context_base_(std::make_shared( - server, validation_visitor, config_message, std::move(drain_manager), - filter_config_provider_manager)), + server, validation_visitor, config_message, std::move(drain_manager))), listener_config_(listener_config), listener_impl_(listener_impl) {} PerListenerFactoryContextImpl( std::shared_ptr listener_factory_context_base, @@ -247,11 +239,6 @@ class PerListenerFactoryContextImpl : public Configuration::ListenerFactoryConte Stats::Scope& listenerScope() override; bool isQuicListener() const override; - Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain) override; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr - downstreamFilterConfigProviderManager() override; // ListenerFactoryContext const Network::ListenerConfig& listenerConfig() const override; diff --git a/source/server/factory_context_impl.cc b/source/server/factory_context_impl.cc index 797be4c239b6..178aa2ee3665 100644 --- a/source/server/factory_context_impl.cc +++ b/source/server/factory_context_impl.cc @@ -63,15 +63,6 @@ envoy::config::core::v3::TrafficDirection FactoryContextImpl::direction() const Network::DrainDecision& FactoryContextImpl::drainDecision() { return drain_decision_; } Stats::Scope& FactoryContextImpl::listenerScope() { return listener_scope_; } bool FactoryContextImpl::isQuicListener() const { return is_quic_; } -Configuration::HttpExtensionConfigProvider -FactoryContextImpl::createHttpDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool) { - return nullptr; -} -Configuration::DownstreamFilterConfigProviderManagerSharedPtr -FactoryContextImpl::downstreamFilterConfigProviderManager() { - return filter_config_provider_manager_; -} } // namespace Server } // namespace Envoy diff --git a/source/server/factory_context_impl.h b/source/server/factory_context_impl.h index 86befe86463f..14ca4e6336cf 100644 --- a/source/server/factory_context_impl.h +++ b/source/server/factory_context_impl.h @@ -49,11 +49,6 @@ class FactoryContextImpl : public Configuration::FactoryContext { Network::DrainDecision& drainDecision() override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; - Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain) override; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr - downstreamFilterConfigProviderManager() override; private: Server::Instance& server_; diff --git a/source/server/server.h b/source/server/server.h index 8ecfa2ccc549..cfb6c69acced 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -172,8 +172,9 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, public Configuration::TransportSocketFactoryContext { public: explicit ServerFactoryContextImpl(Instance& server) - : server_(server), server_scope_(server_.stats().createScope("")) , filter_config_provider_manager_( - std::make_shared()){} + : server_(server), server_scope_(server_.stats().createScope("")), + filter_config_provider_manager_( + std::make_shared()) {} // Configuration::ServerFactoryContext Upstream::ClusterManager& clusterManager() override { return server_.clusterManager(); } @@ -201,12 +202,13 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( Configuration::FactoryContext& factory_context, const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain) override{ - return downstreamFilterConfigProviderManager()->createDynamicFilterConfigProvider( - config_source, filter_config_name, *this, factory_context, clusterManager(), last_filter_in_filter_chain, "http", nullptr); - } + const std::string& filter_config_name, bool last_filter_in_filter_chain) override { + return downstreamFilterConfigProviderManager()->createDynamicFilterConfigProvider( + config_source, filter_config_name, *this, factory_context, clusterManager(), + last_filter_in_filter_chain, "http", nullptr); + } - Configuration::DownstreamFilterConfigProviderManagerSharedPtr + Configuration::DownstreamFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override { return filter_config_provider_manager_; } diff --git a/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc b/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc index 96397f69547f..19e5350af0e0 100644 --- a/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc +++ b/test/extensions/listener_managers/listener_manager/listener_manager_impl_test.cc @@ -5792,8 +5792,6 @@ TEST_P(ListenerManagerImplWithRealFiltersTest, OriginalDstFilter) { EXPECT_EQ(&parent_context.timeSource(), &listener_factory_context->api().timeSource()); EXPECT_EQ(&parent_context.messageValidationContext(), &server_.messageValidationContext()); EXPECT_EQ(&parent_context.lifecycleNotifier(), &server_.lifecycleNotifier()); - envoy::config::core::v3::ExtensionConfigSource config_source; - EXPECT_EQ(parent_context.createHttpDynamicFilterConfigProvider(config_source, "", true), nullptr); Network::FilterChainFactory& filterChainFactory = listener.filterChainFactory(); Network::MockListenerFilterManager manager; diff --git a/test/mocks/server/factory_context.h b/test/mocks/server/factory_context.h index c4654e1fc50f..756160cb1c0c 100644 --- a/test/mocks/server/factory_context.h +++ b/test/mocks/server/factory_context.h @@ -57,16 +57,6 @@ class MockFactoryContext : public virtual ListenerFactoryContext { MOCK_METHOD(ProtobufMessage::ValidationVisitor&, messageValidationVisitor, ()); MOCK_METHOD(Api::Api&, api, ()); - HttpExtensionConfigProvider - createHttpDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, bool) override { - return nullptr; - } - Configuration::DownstreamFilterConfigProviderManagerSharedPtr - downstreamFilterConfigProviderManager() override { - return filter_config_provider_manager_; - } - testing::NiceMock server_factory_context_; testing::NiceMock access_log_manager_; testing::NiceMock cluster_manager_; @@ -92,8 +82,6 @@ class MockFactoryContext : public virtual ListenerFactoryContext { Http::ContextImpl http_context_; Router::ContextImpl router_context_; testing::NiceMock api_; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ - std::make_shared()}; }; class MockUpstreamFactoryContext : public UpstreamFactoryContext { diff --git a/test/mocks/server/listener_factory_context.h b/test/mocks/server/listener_factory_context.h index 9486b4599be6..5341b517d1ba 100644 --- a/test/mocks/server/listener_factory_context.h +++ b/test/mocks/server/listener_factory_context.h @@ -56,17 +56,6 @@ class MockListenerFactoryContext : public ListenerFactoryContext { MOCK_METHOD(ProtobufMessage::ValidationVisitor&, messageValidationVisitor, ()); MOCK_METHOD(Api::Api&, api, ()); - HttpExtensionConfigProvider - createHttpDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, bool) override { - return nullptr; - } - - Configuration::DownstreamFilterConfigProviderManagerSharedPtr - downstreamFilterConfigProviderManager() override { - return nullptr; - } - testing::NiceMock server_factory_context_; testing::NiceMock access_log_manager_; testing::NiceMock cluster_manager_; diff --git a/test/mocks/server/server_factory_context.h b/test/mocks/server/server_factory_context.h index 9b9929a553f8..4c285896c27b 100644 --- a/test/mocks/server/server_factory_context.h +++ b/test/mocks/server/server_factory_context.h @@ -79,8 +79,9 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { MOCK_METHOD(ServerLifecycleNotifier&, lifecycleNotifier, ()); MOCK_METHOD(StatsConfig&, statsConfig, (), ()); MOCK_METHOD(AccessLog::AccessLogManager&, accessLogManager, (), ()); - HttpExtensionConfigProvider - createHttpDynamicFilterConfigProvider( Configuration::FactoryContext&, const envoy::config::core::v3::ExtensionConfigSource&, + HttpExtensionConfigProvider + createHttpDynamicFilterConfigProvider(Configuration::FactoryContext&, + const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool) override { return nullptr; } @@ -110,7 +111,7 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { Router::ContextImpl router_context_; envoy::config::bootstrap::v3::Bootstrap bootstrap_; testing::NiceMock options_; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ std::make_shared()}; }; @@ -145,8 +146,9 @@ class StatelessMockServerFactoryContext : public virtual ServerFactoryContext { MOCK_METHOD(ServerLifecycleNotifier&, lifecycleNotifier, ()); MOCK_METHOD(StatsConfig&, statsConfig, (), ()); MOCK_METHOD(AccessLog::AccessLogManager&, accessLogManager, (), ()); - HttpExtensionConfigProvider - createHttpDynamicFilterConfigProvider( Configuration::FactoryContext&, const envoy::config::core::v3::ExtensionConfigSource&, + HttpExtensionConfigProvider + createHttpDynamicFilterConfigProvider(Configuration::FactoryContext&, + const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool) override { return nullptr; } @@ -154,7 +156,7 @@ class StatelessMockServerFactoryContext : public virtual ServerFactoryContext { downstreamFilterConfigProviderManager() override { return filter_config_provider_manager_; } - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ + Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ std::make_shared()}; }; From 5e164a21c115172e31b5fccd994fc3c03e97d4a7 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sat, 18 Nov 2023 14:11:58 +0530 Subject: [PATCH 61/73] remove unnecessary changes Signed-off-by: Rama Chavali --- .../dynamic_extension_config_provider.h | 1 + source/common/filter/config_discovery_impl.h | 1 + .../filters/http/composite/action.cc | 38 +++++++++---------- .../listener_managers/listener_manager/BUILD | 2 - .../filter_chain_manager_impl.cc | 1 - .../filter_chain_manager_impl.h | 1 - .../listener_manager/listener_impl.cc | 4 -- .../listener_manager/listener_impl.h | 1 - .../listener_manager/listener_manager_impl.cc | 2 - .../listener_manager/listener_manager_impl.h | 1 - 10 files changed, 21 insertions(+), 31 deletions(-) diff --git a/envoy/config/dynamic_extension_config_provider.h b/envoy/config/dynamic_extension_config_provider.h index 4faf1d95b74a..a557ad5fc300 100644 --- a/envoy/config/dynamic_extension_config_provider.h +++ b/envoy/config/dynamic_extension_config_provider.h @@ -51,5 +51,6 @@ class DynamicExtensionConfigProviderBase { template class DynamicExtensionConfigProvider : public DynamicExtensionConfigProviderBase, public ExtensionConfigProvider {}; + } // namespace Config } // namespace Envoy diff --git a/source/common/filter/config_discovery_impl.h b/source/common/filter/config_discovery_impl.h index 7376697cdbf5..7fceb0abae87 100644 --- a/source/common/filter/config_discovery_impl.h +++ b/source/common/filter/config_discovery_impl.h @@ -579,6 +579,7 @@ class FilterConfigProviderManagerImpl : public FilterConfigProviderManagerImplBa getDefaultConfig(config_source.default_config(), filter_config_name, server_context, last_filter_in_filter_chain, filter_chain_type, require_type_urls); } + std::unique_ptr> provider = std::make_unique(subscription, require_type_urls, server_context, factory_context, std::move(default_config), diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 3e3db90450d9..1ca8d4a1642f 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -21,25 +21,7 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( } // Process dynamic filter configuration and setup extension configuration discovery service. - if (composite_action.has_dynamic_config()) { - auto config_discovery = composite_action.dynamic_config().config_discovery(); - Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); - Server::Configuration::ServerFactoryContext& server_factory_context = - context.server_factory_context_.value(); - // Create a dynamic filter config provider and register it with the server factory context. - auto provider = server_factory_context.createHttpDynamicFilterConfigProvider( - factory_context, config_discovery, composite_action.dynamic_config().name(), false); - return [provider = std::move(provider)]() -> Matcher::ActionPtr { - auto config_value = provider->config(); - if (config_value.has_value()) { - auto factory_cb = config_value.value().get().factory_cb; - return std::make_unique(factory_cb); - } - // There is no dynamic config available. Apply missing config filter. - auto factory_cb = Envoy::Http::MissingConfigFilterFactory; - return std::make_unique(factory_cb); - }; - } else { + if (composite_action.has_typed_config()) { // Static filter configuration. auto& factory = Config::Utility::getAndCheckFactory( @@ -66,6 +48,24 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( return [cb = std::move(callback)]() -> Matcher::ActionPtr { return std::make_unique(cb); }; + } else { + auto config_discovery = composite_action.dynamic_config().config_discovery(); + Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); + Server::Configuration::ServerFactoryContext& server_factory_context = + context.server_factory_context_.value(); + // Create a dynamic filter config provider and register it with the server factory context. + auto provider = server_factory_context.createHttpDynamicFilterConfigProvider( + factory_context, config_discovery, composite_action.dynamic_config().name(), false); + return [provider = std::move(provider)]() -> Matcher::ActionPtr { + auto config_value = provider->config(); + if (config_value.has_value()) { + auto factory_cb = config_value.value().get().factory_cb; + return std::make_unique(factory_cb); + } + // There is no dynamic config available. Apply missing config filter. + auto factory_cb = Envoy::Http::MissingConfigFilterFactory; + return std::make_unique(factory_cb); + }; } } diff --git a/source/extensions/listener_managers/listener_manager/BUILD b/source/extensions/listener_managers/listener_manager/BUILD index 2d8d6697f4fe..b8161fabbec1 100644 --- a/source/extensions/listener_managers/listener_manager/BUILD +++ b/source/extensions/listener_managers/listener_manager/BUILD @@ -111,8 +111,6 @@ envoy_cc_library( "//envoy/server:transport_socket_config_interface", "//source/common/common:empty_string", "//source/common/config:utility_lib", - "//source/common/filter:config_discovery_lib", - "//source/common/http:filter_chain_helper_lib", "//source/common/init:manager_lib", "//source/common/matcher:matcher_lib", "//source/common/network:cidr_range_lib", diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc index ec21386f1afb..356072181bb0 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.cc @@ -6,7 +6,6 @@ #include "source/common/common/empty_string.h" #include "source/common/common/fmt.h" #include "source/common/config/utility.h" -#include "source/common/http/filter_chain_helper.h" #include "source/common/matcher/matcher.h" #include "source/common/network/matching/data_impl.h" #include "source/common/network/matching/inputs.h" diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h index 565769438dff..9f42f9286541 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h @@ -17,7 +17,6 @@ #include "envoy/thread_local/thread_local.h" #include "source/common/common/logger.h" -#include "source/common/filter/config_discovery_impl.h" #include "source/common/init/manager_impl.h" #include "source/common/network/cidr_range.h" #include "source/common/network/lc_trie.h" diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.cc b/source/extensions/listener_managers/listener_manager/listener_impl.cc index d0606d05b286..de199ffe5a31 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_impl.cc @@ -299,7 +299,6 @@ ListenerFactoryContextBaseImpl::getTransportSocketFactoryContext() const { } Stats::Scope& ListenerFactoryContextBaseImpl::listenerScope() { return *listener_scope_; } bool ListenerFactoryContextBaseImpl::isQuicListener() const { return is_quic_; } - Network::DrainDecision& ListenerFactoryContextBaseImpl::drainDecision() { return *this; } Server::DrainManager& ListenerFactoryContextBaseImpl::drainManager() { return *drain_manager_; } Init::Manager& ListenerFactoryContextBaseImpl::initManager() { return server_.initManager(); } @@ -920,12 +919,10 @@ const Envoy::Config::TypedMetadata& PerListenerFactoryContextImpl::listenerTyped envoy::config::core::v3::TrafficDirection PerListenerFactoryContextImpl::direction() const { return listener_factory_context_base_->direction(); }; - TimeSource& PerListenerFactoryContextImpl::timeSource() { return api().timeSource(); } const Network::ListenerConfig& PerListenerFactoryContextImpl::listenerConfig() const { return *listener_config_; } - ProtobufMessage::ValidationContext& PerListenerFactoryContextImpl::messageValidationContext() { return getServerFactoryContext().messageValidationContext(); } @@ -953,7 +950,6 @@ Stats::Scope& PerListenerFactoryContextImpl::listenerScope() { bool PerListenerFactoryContextImpl::isQuicListener() const { return listener_factory_context_base_->isQuicListener(); } - Init::Manager& PerListenerFactoryContextImpl::initManager() { return listener_impl_.initManager(); } bool ListenerImpl::createNetworkFilterChain( diff --git a/source/extensions/listener_managers/listener_manager/listener_impl.h b/source/extensions/listener_managers/listener_manager/listener_impl.h index a92e04a415d6..08808e152927 100644 --- a/source/extensions/listener_managers/listener_manager/listener_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_impl.h @@ -19,7 +19,6 @@ #include "source/common/common/basic_resource_impl.h" #include "source/common/common/logger.h" #include "source/common/config/metadata.h" -#include "source/common/filter/config_discovery_impl.h" #include "source/common/init/manager_impl.h" #include "source/common/init/target_impl.h" #include "source/common/quic/quic_stat_names.h" diff --git a/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc b/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc index ff4130595034..66f4f777d5aa 100644 --- a/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc +++ b/source/extensions/listener_managers/listener_manager/listener_manager_impl.cc @@ -350,8 +350,6 @@ ListenerManagerImpl::ListenerManagerImpl(Instance& server, bool enable_dispatcher_stats, Quic::QuicStatNames& quic_stat_names) : server_(server), factory_(std::move(factory)), - filter_config_provider_manager_( - std::make_shared()), scope_(server.stats().createScope("listener_manager.")), stats_(generateStats(*scope_)), enable_dispatcher_stats_(enable_dispatcher_stats), quic_stat_names_(quic_stat_names) { if (!factory_) { diff --git a/source/extensions/listener_managers/listener_manager/listener_manager_impl.h b/source/extensions/listener_managers/listener_manager/listener_manager_impl.h index de7aae308307..56bd78050836 100644 --- a/source/extensions/listener_managers/listener_manager/listener_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/listener_manager_impl.h @@ -236,7 +236,6 @@ class ListenerManagerImpl : public ListenerManager, Logger::Loggable factory_; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{}; private: using ListenerList = std::list; From 645ed326d042dc1bf3362972fceff6270fb04a4e Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sat, 18 Nov 2023 14:27:19 +0530 Subject: [PATCH 62/73] remove more unnecessary changes Signed-off-by: Rama Chavali --- .../extensions/filters/http/composite/v3/composite.proto | 4 ++-- source/extensions/filters/http/composite/action.cc | 3 +-- .../listener_manager/filter_chain_manager_impl.h | 1 + 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/envoy/extensions/filters/http/composite/v3/composite.proto b/api/envoy/extensions/filters/http/composite/v3/composite.proto index d4ec02001049..a6132b7043dd 100644 --- a/api/envoy/extensions/filters/http/composite/v3/composite.proto +++ b/api/envoy/extensions/filters/http/composite/v3/composite.proto @@ -38,8 +38,8 @@ message DynamicConfig { string name = 1 [(validate.rules).string = {min_len: 1}]; // Configuration source specifier for an extension configuration discovery - // service. In case of a failure and without the default configuration, the - // listener closes the connections. + // service. In case of a failure and without the default configuration, + // 500(Internal Server Error) will be returned. config.core.v3.ExtensionConfigSource config_discovery = 2; } diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 1ca8d4a1642f..f9613293218e 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -20,7 +20,6 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( fmt::format("Error: Only one of `dynamic_config` or `typed_config` can be set.")); } - // Process dynamic filter configuration and setup extension configuration discovery service. if (composite_action.has_typed_config()) { // Static filter configuration. auto& factory = @@ -49,11 +48,11 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( return std::make_unique(cb); }; } else { + // Create a dynamic filter config provider and register it with the server factory context. auto config_discovery = composite_action.dynamic_config().config_discovery(); Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); - // Create a dynamic filter config provider and register it with the server factory context. auto provider = server_factory_context.createHttpDynamicFilterConfigProvider( factory_context, config_discovery, composite_action.dynamic_config().name(), false); return [provider = std::move(provider)]() -> Matcher::ActionPtr { diff --git a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h index 9f42f9286541..7addd4f710ff 100644 --- a/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h +++ b/source/extensions/listener_managers/listener_manager/filter_chain_manager_impl.h @@ -89,6 +89,7 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor Configuration::TransportSocketFactoryContext& getTransportSocketFactoryContext() const override; Stats::Scope& listenerScope() override; bool isQuicListener() const override; + void startDraining() override { is_draining_.store(true); } private: From 81c70591eccece51bcd2d63b27c6cb47d7934e11 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sat, 18 Nov 2023 15:35:00 +0530 Subject: [PATCH 63/73] format and move to shared ptr Signed-off-by: Rama Chavali --- envoy/server/factory_context.h | 8 ++++---- source/server/server.h | 2 +- test/mocks/server/filter_chain_factory_context.h | 2 +- test/mocks/server/server_factory_context.h | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index c84ef0410467..b8c1352c4fb4 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -43,7 +43,7 @@ template class FilterConfigProviderManager; namespace Server { namespace Configuration { -using HttpExtensionConfigProvider = +using HttpExtensionConfigProviderSharedPtr = std::shared_ptr>; // Shared factory context between server factories and cluster factories @@ -198,7 +198,7 @@ class ServerFactoryContext : public virtual CommonFactoryContext { */ virtual envoy::config::bootstrap::v3::Bootstrap& bootstrap() PURE; - /** + /** * @return OverloadManager& the overload manager for the server. */ virtual OverloadManager& overloadManager() PURE; @@ -217,9 +217,9 @@ class ServerFactoryContext : public virtual CommonFactoryContext { * @param last_filter_in_filter_chain indicates whether this filter is the last filter in the * configured chain * - * @return HttpExtensionConfigProvider + * @return HttpExtensionConfigProviderSharedPtr */ - virtual Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( + virtual Configuration::HttpExtensionConfigProviderSharedPtr createHttpDynamicFilterConfigProvider( Configuration::FactoryContext& factory_context, const envoy::config::core::v3::ExtensionConfigSource& config_source, const std::string& filter_config_name, bool last_filter_in_filter_chain) PURE; diff --git a/source/server/server.h b/source/server/server.h index 24dcef48c10b..c51b81bbbee9 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -202,7 +202,7 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, envoy::config::bootstrap::v3::Bootstrap& bootstrap() override { return server_.bootstrap(); } OverloadManager& overloadManager() override { return server_.overloadManager(); } bool healthCheckFailed() const override { return server_.healthCheckFailed(); } - Configuration::HttpExtensionConfigProvider createHttpDynamicFilterConfigProvider( + Configuration::HttpExtensionConfigProviderSharedPtr createHttpDynamicFilterConfigProvider( Configuration::FactoryContext& factory_context, const envoy::config::core::v3::ExtensionConfigSource& config_source, const std::string& filter_config_name, bool last_filter_in_filter_chain) override { diff --git a/test/mocks/server/filter_chain_factory_context.h b/test/mocks/server/filter_chain_factory_context.h index 327c29f7c575..c9f71c5d3686 100644 --- a/test/mocks/server/filter_chain_factory_context.h +++ b/test/mocks/server/filter_chain_factory_context.h @@ -12,7 +12,7 @@ class MockFilterChainFactoryContext : public MockFactoryContext, public FilterCh MockFilterChainFactoryContext(); ~MockFilterChainFactoryContext() override; - HttpExtensionConfigProvider + HttpExtensionConfigProviderSharedPtr createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool) override { return nullptr; diff --git a/test/mocks/server/server_factory_context.h b/test/mocks/server/server_factory_context.h index 11283755f181..5144467b89e3 100644 --- a/test/mocks/server/server_factory_context.h +++ b/test/mocks/server/server_factory_context.h @@ -82,7 +82,7 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { MOCK_METHOD(AccessLog::AccessLogManager&, accessLogManager, (), ()); MOCK_METHOD(OverloadManager&, overloadManager, ()); MOCK_METHOD(bool, healthCheckFailed, (), (const)); - HttpExtensionConfigProvider + HttpExtensionConfigProviderSharedPtr createHttpDynamicFilterConfigProvider(Configuration::FactoryContext&, const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool) override { @@ -92,7 +92,7 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { downstreamFilterConfigProviderManager() override { return filter_config_provider_manager_; } - + testing::NiceMock cluster_manager_; testing::NiceMock dispatcher_; testing::NiceMock drain_manager_; @@ -154,7 +154,7 @@ class StatelessMockServerFactoryContext : public virtual ServerFactoryContext { MOCK_METHOD(AccessLog::AccessLogManager&, accessLogManager, (), ()); MOCK_METHOD(OverloadManager&, overloadManager, ()); MOCK_METHOD(bool, healthCheckFailed, (), (const)); - HttpExtensionConfigProvider + HttpExtensionConfigProviderSharedPtr createHttpDynamicFilterConfigProvider(Configuration::FactoryContext&, const envoy::config::core::v3::ExtensionConfigSource&, const std::string&, bool) override { From bf8b8e959f3b14ec09e064deabd71fffc3992ce1 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Sat, 18 Nov 2023 17:07:59 +0530 Subject: [PATCH 64/73] remove file change Signed-off-by: Rama Chavali --- test/mocks/server/filter_chain_factory_context.h | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test/mocks/server/filter_chain_factory_context.h b/test/mocks/server/filter_chain_factory_context.h index c9f71c5d3686..f09e7a565722 100644 --- a/test/mocks/server/filter_chain_factory_context.h +++ b/test/mocks/server/filter_chain_factory_context.h @@ -11,16 +11,6 @@ class MockFilterChainFactoryContext : public MockFactoryContext, public FilterCh public: MockFilterChainFactoryContext(); ~MockFilterChainFactoryContext() override; - - HttpExtensionConfigProviderSharedPtr - createDynamicFilterConfigProvider(const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, bool) override { - return nullptr; - } - Configuration::DownstreamFilterConfigProviderManagerSharedPtr - downstreamFilterConfigProviderManager() override { - return nullptr; - } }; } // namespace Configuration } // namespace Server From e83f7a7dd9b42db25fbd81ae496aa4487fb9c544 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 21 Nov 2023 11:20:17 +0530 Subject: [PATCH 65/73] address more review comments Signed-off-by: Rama Chavali --- envoy/server/factory_context.h | 26 ++----- .../filters/http/composite/action.cc | 67 ++++++++++--------- .../network/http_connection_manager/config.cc | 2 +- source/server/factory_context_impl.h | 2 +- source/server/server.h | 13 +--- test/mocks/server/server_factory_context.h | 20 ++---- 6 files changed, 51 insertions(+), 79 deletions(-) diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index b8c1352c4fb4..4dda788056ef 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -153,11 +153,11 @@ class CommonFactoryContext : public FactoryContextBase { class FactoryContext; -using DownstreamFilterConfigProviderManager = +using DownstreamHTTPFilterConfigProviderManager = Filter::FilterConfigProviderManager; -using DownstreamFilterConfigProviderManagerSharedPtr = - std::shared_ptr; +using DownstreamHTTPFilterConfigProviderManagerSharedPtr = + std::shared_ptr; /** * ServerFactoryContext is an specialization of common interface for downstream and upstream network @@ -208,28 +208,12 @@ class ServerFactoryContext : public virtual CommonFactoryContext { */ virtual bool healthCheckFailed() const PURE; - /** - * Create an FilterConfigProviderPtr for a filter config. The config providers may share - * the underlying subscriptions to the filter config discovery service. - * @param factory_context supplies the factory context. - * @param config_source supplies the extension configuration source for the filter configs. - * @param filter_config_name the filter config resource name. - * @param last_filter_in_filter_chain indicates whether this filter is the last filter in the - * configured chain - * - * @return HttpExtensionConfigProviderSharedPtr - */ - virtual Configuration::HttpExtensionConfigProviderSharedPtr createHttpDynamicFilterConfigProvider( - Configuration::FactoryContext& factory_context, - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain) PURE; - /** * Returns the downstream filter config provider manager. * - * @return DownstreamFilterConfigProviderManagerSharedPtr + * @return DownstreamHTTPFilterConfigProviderManagerSharedPtr */ - virtual DownstreamFilterConfigProviderManagerSharedPtr + virtual DownstreamHTTPFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() PURE; }; diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index f9613293218e..60cf19147dbe 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -20,41 +20,17 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( fmt::format("Error: Only one of `dynamic_config` or `typed_config` can be set.")); } - if (composite_action.has_typed_config()) { - // Static filter configuration. - auto& factory = - Config::Utility::getAndCheckFactory( - composite_action.typed_config()); - ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig( - composite_action.typed_config().typed_config(), validation_visitor, factory); - Envoy::Http::FilterFactoryCb callback = nullptr; - - // First, try to create the filter factory creation function from factory context (if exists). - if (context.factory_context_.has_value()) { - auto callback_or_status = factory.createFilterFactoryFromProto( - *message, context.stat_prefix_, context.factory_context_.value()); - THROW_IF_STATUS_NOT_OK(callback_or_status, throw); - callback = callback_or_status.value(); - } - - // If above failed, try to create the filter factory creation function from server factory - // context (if exists). - if (callback == nullptr && context.server_factory_context_.has_value()) { - callback = factory.createFilterFactoryFromProtoWithServerContext( - *message, context.stat_prefix_, context.server_factory_context_.value()); - } - - return [cb = std::move(callback)]() -> Matcher::ActionPtr { - return std::make_unique(cb); - }; - } else { + if (composite_action.has_dynamic_config()) { // Create a dynamic filter config provider and register it with the server factory context. auto config_discovery = composite_action.dynamic_config().config_discovery(); Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); - auto provider = server_factory_context.createHttpDynamicFilterConfigProvider( - factory_context, config_discovery, composite_action.dynamic_config().name(), false); + Server::Configuration::HttpExtensionConfigProviderSharedPtr provider = + server_factory_context.downstreamFilterConfigProviderManager() + ->createDynamicFilterConfigProvider( + config_discovery, composite_action.dynamic_config().name(), server_factory_context, + factory_context, server_factory_context.clusterManager(), false, "http", nullptr); return [provider = std::move(provider)]() -> Matcher::ActionPtr { auto config_value = provider->config(); if (config_value.has_value()) { @@ -66,6 +42,37 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( return std::make_unique(factory_cb); }; } + + auto& factory = + Config::Utility::getAndCheckFactory( + composite_action.typed_config()); + ProtobufTypes::MessagePtr message = Config::Utility::translateAnyToFactoryConfig( + composite_action.typed_config().typed_config(), validation_visitor, factory); + + Envoy::Http::FilterFactoryCb callback = nullptr; + + // First, try to create the filter factory creation function from factory context (if exists). + if (context.factory_context_.has_value()) { + auto callback_or_status = factory.createFilterFactoryFromProto( + *message, context.stat_prefix_, context.factory_context_.value()); + THROW_IF_STATUS_NOT_OK(callback_or_status, throw); + callback = callback_or_status.value(); + } + + // If above failed, try to create the filter factory creation function from server factory + // context (if exists). + if (callback == nullptr && context.server_factory_context_.has_value()) { + callback = factory.createFilterFactoryFromProtoWithServerContext( + *message, context.stat_prefix_, context.server_factory_context_.value()); + } + + if (callback == nullptr) { + throwEnvoyExceptionOrPanic("Failed to get filter factory creation function"); + } + + return [cb = std::move(callback)]() -> Matcher::ActionPtr { + return std::make_unique(cb); + }; } REGISTER_FACTORY(ExecuteFilterActionFactory, diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index abe15abfc2c0..1edf4b5c90b6 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -231,7 +231,7 @@ Utility::Singletons Utility::createSingletons(Server::Configuration::FactoryCont auto tracer_manager = Tracing::TracerManagerImpl::singleton(context); - std::shared_ptr + Server::Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr filter_config_provider_manager = context.getServerFactoryContext().downstreamFilterConfigProviderManager(); diff --git a/source/server/factory_context_impl.h b/source/server/factory_context_impl.h index 14ca4e6336cf..ab4597faa668 100644 --- a/source/server/factory_context_impl.h +++ b/source/server/factory_context_impl.h @@ -57,7 +57,7 @@ class FactoryContextImpl : public Configuration::FactoryContext { Stats::Scope& global_scope_; Stats::Scope& listener_scope_; bool is_quic_; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{}; + Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr filter_config_provider_manager_; }; } // namespace Server diff --git a/source/server/server.h b/source/server/server.h index c51b81bbbee9..aa6ebec5ff47 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -202,15 +202,7 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, envoy::config::bootstrap::v3::Bootstrap& bootstrap() override { return server_.bootstrap(); } OverloadManager& overloadManager() override { return server_.overloadManager(); } bool healthCheckFailed() const override { return server_.healthCheckFailed(); } - Configuration::HttpExtensionConfigProviderSharedPtr createHttpDynamicFilterConfigProvider( - Configuration::FactoryContext& factory_context, - const envoy::config::core::v3::ExtensionConfigSource& config_source, - const std::string& filter_config_name, bool last_filter_in_filter_chain) override { - return downstreamFilterConfigProviderManager()->createDynamicFilterConfigProvider( - config_source, filter_config_name, *this, factory_context, clusterManager(), - last_filter_in_filter_chain, "http", nullptr); - } - Configuration::DownstreamFilterConfigProviderManagerSharedPtr + Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override { return filter_config_provider_manager_; } @@ -235,7 +227,8 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, private: Instance& server_; Stats::ScopeSharedPtr server_scope_; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{}; + Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr + filter_config_provider_manager_{}; }; /** diff --git a/test/mocks/server/server_factory_context.h b/test/mocks/server/server_factory_context.h index 5144467b89e3..c9df71b25b3b 100644 --- a/test/mocks/server/server_factory_context.h +++ b/test/mocks/server/server_factory_context.h @@ -82,13 +82,7 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { MOCK_METHOD(AccessLog::AccessLogManager&, accessLogManager, (), ()); MOCK_METHOD(OverloadManager&, overloadManager, ()); MOCK_METHOD(bool, healthCheckFailed, (), (const)); - HttpExtensionConfigProviderSharedPtr - createHttpDynamicFilterConfigProvider(Configuration::FactoryContext&, - const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, bool) override { - return nullptr; - } - Configuration::DownstreamFilterConfigProviderManagerSharedPtr + Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override { return filter_config_provider_manager_; } @@ -116,7 +110,7 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { Router::ContextImpl router_context_; envoy::config::bootstrap::v3::Bootstrap bootstrap_; testing::NiceMock options_; - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ + Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ std::make_shared()}; }; @@ -154,17 +148,11 @@ class StatelessMockServerFactoryContext : public virtual ServerFactoryContext { MOCK_METHOD(AccessLog::AccessLogManager&, accessLogManager, (), ()); MOCK_METHOD(OverloadManager&, overloadManager, ()); MOCK_METHOD(bool, healthCheckFailed, (), (const)); - HttpExtensionConfigProviderSharedPtr - createHttpDynamicFilterConfigProvider(Configuration::FactoryContext&, - const envoy::config::core::v3::ExtensionConfigSource&, - const std::string&, bool) override { - return nullptr; - } - Configuration::DownstreamFilterConfigProviderManagerSharedPtr + Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr downstreamFilterConfigProviderManager() override { return filter_config_provider_manager_; } - Configuration::DownstreamFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ + Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ std::make_shared()}; }; From e17e912e22d896ba28bd95ec960c201a526b7023 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Wed, 22 Nov 2023 09:56:18 +0530 Subject: [PATCH 66/73] some more review comments Signed-off-by: Rama Chavali --- envoy/server/factory_context.h | 4 ++-- source/extensions/filters/http/composite/action.cc | 6 +++++- .../filters/network/http_connection_manager/config.cc | 2 +- source/server/server.h | 2 +- .../http/composite/composite_filter_integration_test.cc | 2 -- test/mocks/server/server_factory_context.h | 4 ++-- 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index 4dda788056ef..849a0191d5a1 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -209,12 +209,12 @@ class ServerFactoryContext : public virtual CommonFactoryContext { virtual bool healthCheckFailed() const PURE; /** - * Returns the downstream filter config provider manager. + * Returns the downstream HTTP filter config provider manager. * * @return DownstreamHTTPFilterConfigProviderManagerSharedPtr */ virtual DownstreamHTTPFilterConfigProviderManagerSharedPtr - downstreamFilterConfigProviderManager() PURE; + downstreamHttpFilterConfigProviderManager() PURE; }; /** diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 60cf19147dbe..49d78f7e2290 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -21,13 +21,17 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( } if (composite_action.has_dynamic_config()) { + if (!context.factory_context_.has_value() || !context.server_factory_context_.has_value()) { + throwEnvoyExceptionOrPanic( + fmt::format("Failed to get factory context or server factory context.")); + } // Create a dynamic filter config provider and register it with the server factory context. auto config_discovery = composite_action.dynamic_config().config_discovery(); Server::Configuration::FactoryContext& factory_context = context.factory_context_.value(); Server::Configuration::ServerFactoryContext& server_factory_context = context.server_factory_context_.value(); Server::Configuration::HttpExtensionConfigProviderSharedPtr provider = - server_factory_context.downstreamFilterConfigProviderManager() + server_factory_context.downstreamHttpFilterConfigProviderManager() ->createDynamicFilterConfigProvider( config_discovery, composite_action.dynamic_config().name(), server_factory_context, factory_context, server_factory_context.clusterManager(), false, "http", nullptr); diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 1edf4b5c90b6..afb9411f75b1 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -233,7 +233,7 @@ Utility::Singletons Utility::createSingletons(Server::Configuration::FactoryCont Server::Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr filter_config_provider_manager = - context.getServerFactoryContext().downstreamFilterConfigProviderManager(); + context.getServerFactoryContext().downstreamHttpFilterConfigProviderManager(); return {date_provider, route_config_provider_manager, scoped_routes_config_provider_manager, tracer_manager, filter_config_provider_manager}; diff --git a/source/server/server.h b/source/server/server.h index aa6ebec5ff47..9e8582e0916b 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -203,7 +203,7 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, OverloadManager& overloadManager() override { return server_.overloadManager(); } bool healthCheckFailed() const override { return server_.healthCheckFailed(); } Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr - downstreamFilterConfigProviderManager() override { + downstreamHttpFilterConfigProviderManager() override { return filter_config_provider_manager_; } diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index f97f0a236cc0..8521c3666de8 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -177,8 +177,6 @@ class CompositeFilterIntegrationTest : public testing::TestWithParam Date: Wed, 22 Nov 2023 10:17:43 +0530 Subject: [PATCH 67/73] remove factory context Signed-off-by: Rama Chavali --- source/server/factory_context_impl.cc | 4 +--- source/server/factory_context_impl.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/source/server/factory_context_impl.cc b/source/server/factory_context_impl.cc index 178aa2ee3665..3d525653814c 100644 --- a/source/server/factory_context_impl.cc +++ b/source/server/factory_context_impl.cc @@ -9,9 +9,7 @@ FactoryContextImpl::FactoryContextImpl(Server::Instance& server, Stats::Scope& global_scope, Stats::Scope& listener_scope, bool is_quic) : server_(server), config_(config), drain_decision_(drain_decision), - global_scope_(global_scope), listener_scope_(listener_scope), is_quic_(is_quic), - filter_config_provider_manager_( - std::make_shared()) {} + global_scope_(global_scope), listener_scope_(listener_scope), is_quic_(is_quic) {} AccessLog::AccessLogManager& FactoryContextImpl::accessLogManager() { return server_.accessLogManager(); diff --git a/source/server/factory_context_impl.h b/source/server/factory_context_impl.h index ab4597faa668..c61876403129 100644 --- a/source/server/factory_context_impl.h +++ b/source/server/factory_context_impl.h @@ -57,7 +57,6 @@ class FactoryContextImpl : public Configuration::FactoryContext { Stats::Scope& global_scope_; Stats::Scope& listener_scope_; bool is_quic_; - Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr filter_config_provider_manager_; }; } // namespace Server From 4bcfd94201add56aee8b1a0638ecdbf622da5b9c Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 23 Nov 2023 08:58:03 +0530 Subject: [PATCH 68/73] nits Signed-off-by: Rama Chavali --- source/server/factory_context_impl.h | 2 -- source/server/server.h | 3 +-- test/mocks/server/server_factory_context.cc | 8 +++++++- test/mocks/server/server_factory_context.h | 5 ++--- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/source/server/factory_context_impl.h b/source/server/factory_context_impl.h index c61876403129..5fdfd022e5b0 100644 --- a/source/server/factory_context_impl.h +++ b/source/server/factory_context_impl.h @@ -3,8 +3,6 @@ #include "envoy/server/factory_context.h" #include "envoy/server/instance.h" -#include "source/common/filter/config_discovery_impl.h" - namespace Envoy { namespace Server { diff --git a/source/server/server.h b/source/server/server.h index 9e8582e0916b..560a6621b775 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -227,8 +227,7 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, private: Instance& server_; Stats::ScopeSharedPtr server_scope_; - Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr - filter_config_provider_manager_{}; + Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr filter_config_provider_manager_; }; /** diff --git a/test/mocks/server/server_factory_context.cc b/test/mocks/server/server_factory_context.cc index e8d184fe410d..6c98f0ecc168 100644 --- a/test/mocks/server/server_factory_context.cc +++ b/test/mocks/server/server_factory_context.cc @@ -10,7 +10,9 @@ using ::testing::ReturnRef; MockServerFactoryContext::MockServerFactoryContext() : singleton_manager_(new Singleton::ManagerImpl(Thread::threadFactoryForTest())), http_context_(store_.symbolTable()), grpc_context_(store_.symbolTable()), - router_context_(store_.symbolTable()) { + router_context_(store_.symbolTable()), + filter_config_provider_manager_( + std::make_shared()) { ON_CALL(*this, clusterManager()).WillByDefault(ReturnRef(cluster_manager_)); ON_CALL(*this, mainThreadDispatcher()).WillByDefault(ReturnRef(dispatcher_)); ON_CALL(*this, drainDecision()).WillByDefault(ReturnRef(drain_manager_)); @@ -40,6 +42,10 @@ MockServerFactoryContext::~MockServerFactoryContext() = default; MockStatsConfig::MockStatsConfig() = default; MockStatsConfig::~MockStatsConfig() = default; +StatelessMockServerFactoryContext::StatelessMockServerFactoryContext() + : filter_config_provider_manager_( + std::make_shared()) {} + } // namespace Configuration } // namespace Server } // namespace Envoy diff --git a/test/mocks/server/server_factory_context.h b/test/mocks/server/server_factory_context.h index 3efd5c6cc402..beabc760111b 100644 --- a/test/mocks/server/server_factory_context.h +++ b/test/mocks/server/server_factory_context.h @@ -118,7 +118,7 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { // threads. Global state in the MockServerFactoryContext causes thread safety issues in this case. class StatelessMockServerFactoryContext : public virtual ServerFactoryContext { public: - StatelessMockServerFactoryContext() = default; + StatelessMockServerFactoryContext(); ~StatelessMockServerFactoryContext() override = default; MOCK_METHOD(Upstream::ClusterManager&, clusterManager, ()); @@ -152,8 +152,7 @@ class StatelessMockServerFactoryContext : public virtual ServerFactoryContext { downstreamHttpFilterConfigProviderManager() override { return filter_config_provider_manager_; } - Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr filter_config_provider_manager_{ - std::make_shared()}; + Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr filter_config_provider_manager_; }; } // namespace Configuration From 1093d7711f831dade3b50f46634711781f41ca2b Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 30 Nov 2023 23:48:21 +0530 Subject: [PATCH 69/73] move to envoy exception Signed-off-by: Rama Chavali --- source/extensions/filters/http/composite/action.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/http/composite/action.cc b/source/extensions/filters/http/composite/action.cc index 6456a2ff6c03..2d7e6edd56b9 100644 --- a/source/extensions/filters/http/composite/action.cc +++ b/source/extensions/filters/http/composite/action.cc @@ -16,14 +16,13 @@ Matcher::ActionFactoryCb ExecuteFilterActionFactory::createActionFactoryCb( config, validation_visitor); if (composite_action.has_dynamic_config() && composite_action.has_typed_config()) { - throwEnvoyExceptionOrPanic( + throw EnvoyException( fmt::format("Error: Only one of `dynamic_config` or `typed_config` can be set.")); } if (composite_action.has_dynamic_config()) { if (!context.factory_context_.has_value() || !context.server_factory_context_.has_value()) { - throwEnvoyExceptionOrPanic( - fmt::format("Failed to get factory context or server factory context.")); + throw EnvoyException(fmt::format("Failed to get factory context or server factory context.")); } // Create a dynamic filter config provider and register it with the server factory context. auto config_discovery = composite_action.dynamic_config().config_discovery(); From 6e97ce122c1582589f1b6eaaced5780320c99138 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 5 Dec 2023 13:55:21 +0530 Subject: [PATCH 70/73] format Signed-off-by: Rama Chavali --- test/mocks/server/server_factory_context.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mocks/server/server_factory_context.cc b/test/mocks/server/server_factory_context.cc index f7e62cdb3b37..c9bfdc5d153f 100644 --- a/test/mocks/server/server_factory_context.cc +++ b/test/mocks/server/server_factory_context.cc @@ -45,7 +45,7 @@ MockStatsConfig::~MockStatsConfig() = default; StatelessMockServerFactoryContext::StatelessMockServerFactoryContext() : filter_config_provider_manager_( std::make_shared()) {} - + MockGenericFactoryContext::~MockGenericFactoryContext() = default; MockGenericFactoryContext::MockGenericFactoryContext() { From 25f4eab4cc5a05b44c36fcebdba9a29e1be14031 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 5 Dec 2023 22:46:59 +0530 Subject: [PATCH 71/73] move to serverfactory context Signed-off-by: Rama Chavali --- .../filters/network/http_connection_manager/config.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 1771184176ad..10aa8b32b74b 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -233,7 +233,7 @@ Utility::Singletons Utility::createSingletons(Server::Configuration::FactoryCont Server::Configuration::DownstreamHTTPFilterConfigProviderManagerSharedPtr filter_config_provider_manager = - context.getServerFactoryContext().downstreamHttpFilterConfigProviderManager(); + context.serverFactoryContext().downstreamHttpFilterConfigProviderManager(); return {date_provider, route_config_provider_manager, scoped_routes_config_provider_manager, tracer_manager, filter_config_provider_manager}; From 4641d004b2eb68c2f3600f565d8db306294726b7 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Fri, 8 Dec 2023 10:10:44 +0530 Subject: [PATCH 72/73] format Signed-off-by: Rama Chavali --- source/server/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server/BUILD b/source/server/BUILD index 6c14e33da333..6cdecb25bd94 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -359,8 +359,8 @@ envoy_cc_library( deps = [ "//envoy/server:factory_context_interface", "//envoy/server:instance_interface", - "//source/common/filter:config_discovery_lib", "//source/common/config:metadata_lib", + "//source/common/filter:config_discovery_lib", "//source/common/listener_manager:listener_info_lib", ], ) From 68e8f1da3db57b85c4f8bfafbaed739391997563 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Fri, 8 Dec 2023 13:32:16 +0530 Subject: [PATCH 73/73] kick ci Signed-off-by: Rama Chavali