diff --git a/source/common/filter/http/filter_config_discovery_impl.cc b/source/common/filter/http/filter_config_discovery_impl.cc index eccab3fe988d3..42002c1b7c58f 100644 --- a/source/common/filter/http/filter_config_discovery_impl.cc +++ b/source/common/filter/http/filter_config_discovery_impl.cc @@ -53,12 +53,18 @@ void DynamicFilterConfigProviderImpl::validateConfig( void DynamicFilterConfigProviderImpl::onConfigUpdate(Envoy::Http::FilterFactoryCb config, const std::string&) { - tls_->runOnAllThreads([config](ThreadLocal::ThreadLocalObjectSharedPtr previous) - -> ThreadLocal::ThreadLocalObjectSharedPtr { - auto prev_config = std::dynamic_pointer_cast(previous); - prev_config->config_ = config; - return previous; - }); + tls_->runOnAllThreads( + [config](ThreadLocal::ThreadLocalObjectSharedPtr previous) + -> ThreadLocal::ThreadLocalObjectSharedPtr { + auto prev_config = std::dynamic_pointer_cast(previous); + prev_config->config_ = config; + return previous; + }, + [this, config]() { + // This happens after all workers have discarded the previous config so it can be safely + // deleted on the main thread by an update with the new config. + this->current_config_ = config; + }); } FilterConfigSubscription::FilterConfigSubscription( diff --git a/source/common/filter/http/filter_config_discovery_impl.h b/source/common/filter/http/filter_config_discovery_impl.h index 626832dd8a11b..57aea57b28f99 100644 --- a/source/common/filter/http/filter_config_discovery_impl.h +++ b/source/common/filter/http/filter_config_discovery_impl.h @@ -51,6 +51,9 @@ class DynamicFilterConfigProviderImpl : public FilterConfigProvider { FilterConfigSubscriptionSharedPtr subscription_; const std::set require_type_urls_; + // Currently applied configuration to ensure that the main thread deletes the last reference to + // it. + absl::optional current_config_{absl::nullopt}; ThreadLocal::SlotPtr tls_; // Local initialization target to ensure that the subscription starts in diff --git a/test/integration/BUILD b/test/integration/BUILD index 12fdbd25532fc..1e4176d19a824 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -858,10 +858,10 @@ envoy_cc_test( tags = ["fails_on_windows"], deps = [ ":http_integration_lib", - "//source/extensions/filters/http/rbac:config", "//test/common/grpc:grpc_client_integration_lib", + "//test/integration/filters:set_response_code_filter_config_proto_cc_proto", + "//test/integration/filters:set_response_code_filter_lib", "//test/test_common:utility_lib", - "@envoy_api//envoy/extensions/filters/http/rbac/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", "@envoy_api//envoy/service/extension/v3:pkg_cc_proto", ], diff --git a/test/integration/extension_discovery_integration_test.cc b/test/integration/extension_discovery_integration_test.cc index 0a0fa4559ec7e..467922b3e1236 100644 --- a/test/integration/extension_discovery_integration_test.cc +++ b/test/integration/extension_discovery_integration_test.cc @@ -1,8 +1,8 @@ -#include "envoy/extensions/filters/http/rbac/v3/rbac.pb.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" #include "envoy/service/extension/v3/config_discovery.pb.h" #include "test/common/grpc/grpc_client_integration.h" +#include "test/integration/filters/set_response_code_filter_config.pb.h" #include "test/integration/http_integration.h" #include "test/test_common/utility.h" @@ -13,38 +13,14 @@ namespace { std::string denyPrivateConfig() { return R"EOF( - rules: - action: DENY - policies: - "test": - permissions: - - url_path: { path: { prefix: "/private" } } - principals: - - any: true + prefix: "/private" + code: 403 )EOF"; } -std::string allowAllConfig() { - return R"EOF( - rules: - action: ALLOW - policies: - "test": - permissions: - - any: true - principals: - - any: true -)EOF"; -} +std::string allowAllConfig() { return "code: 200"; } -std::string invalidConfig() { - return R"EOF( - rules: - action: DENY - policies: - "test": {} -)EOF"; -} +std::string invalidConfig() { return "code: 90"; } class ExtensionDiscoveryIntegrationTest : public Grpc::GrpcClientIntegrationParamTest, public HttpIntegrationTest { @@ -62,20 +38,12 @@ class ExtensionDiscoveryIntegrationTest : public Grpc::GrpcClientIntegrationPara filter->set_name(name); auto* discovery = filter->mutable_config_discovery(); discovery->add_type_urls( - "type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC"); + "type.googleapis.com/test.integration.filters.SetResponseCodeFilterConfig"); if (set_default_config) { - const auto rbac_configuration = - TestUtility::parseYaml(R"EOF( - rules: - action: DENY - policies: - "test": - permissions: - - any: true - principals: - - any: true - )EOF"); - discovery->mutable_default_config()->PackFrom(rbac_configuration); + const auto default_configuration = + TestUtility::parseYaml( + "code: 403"); + discovery->mutable_default_config()->PackFrom(default_configuration); } discovery->set_apply_default_config_without_warming(apply_without_warming); auto* api_config_source = discovery->mutable_config_source()->mutable_api_config_source(); @@ -145,15 +113,16 @@ class ExtensionDiscoveryIntegrationTest : public Grpc::GrpcClientIntegrationPara } void sendXdsResponse(const std::string& name, const std::string& version, - const std::string& rbac_config) { + const std::string& yaml_config) { envoy::service::discovery::v3::DiscoveryResponse response; response.set_version_info(version); response.set_type_url("type.googleapis.com/envoy.config.core.v3.TypedExtensionConfig"); - const auto rbac_configuration = - TestUtility::parseYaml(rbac_config); + const auto configuration = + TestUtility::parseYaml( + yaml_config); envoy::config::core::v3::TypedExtensionConfig typed_config; typed_config.set_name(name); - typed_config.mutable_typed_config()->PackFrom(rbac_configuration); + typed_config.mutable_typed_config()->PackFrom(configuration); response.add_resources()->PackFrom(typed_config); ecds_stream_->sendGrpcMessage(response); } diff --git a/test/integration/filters/BUILD b/test/integration/filters/BUILD index 516b02f8c1004..197dfc897cfce 100644 --- a/test/integration/filters/BUILD +++ b/test/integration/filters/BUILD @@ -2,6 +2,7 @@ load( "//bazel:envoy_build_system.bzl", "envoy_cc_test_library", "envoy_package", + "envoy_proto_library", ) licenses(["notice"]) # Apache 2 @@ -172,6 +173,25 @@ envoy_cc_test_library( ], ) +envoy_cc_test_library( + name = "set_response_code_filter_lib", + srcs = [ + "set_response_code_filter.cc", + ], + deps = [ + ":set_response_code_filter_config_proto_cc_proto", + "//include/envoy/http:filter_interface", + "//include/envoy/registry", + "//source/extensions/filters/http/common:factory_base_lib", + "//source/extensions/filters/http/common:pass_through_filter_lib", + ], +) + +envoy_proto_library( + name = "set_response_code_filter_config_proto", + srcs = [":set_response_code_filter_config.proto"], +) + envoy_cc_test_library( name = "stop_iteration_and_continue", srcs = [ diff --git a/test/integration/filters/set_response_code_filter.cc b/test/integration/filters/set_response_code_filter.cc new file mode 100644 index 0000000000000..28653c0ba080b --- /dev/null +++ b/test/integration/filters/set_response_code_filter.cc @@ -0,0 +1,64 @@ +#include + +#include "envoy/http/filter.h" +#include "envoy/registry/registry.h" + +#include "extensions/filters/http/common/factory_base.h" +#include "extensions/filters/http/common/pass_through_filter.h" + +#include "test/integration/filters/set_response_code_filter_config.pb.h" +#include "test/integration/filters/set_response_code_filter_config.pb.validate.h" + +#include "absl/strings/match.h" + +namespace Envoy { + +// A test filter that responds directly with a code on a prefix match. +class SetResponseCodeFilterConfig { +public: + SetResponseCodeFilterConfig(const std::string& prefix, uint32_t code, + Server::Configuration::FactoryContext& context) + : prefix_(prefix), code_(code), tls_slot_(context.threadLocal().allocateSlot()) {} + + const std::string prefix_; + const uint32_t code_; + // Allocate a slot to validate that it is destroyed on a main thread only. + ThreadLocal::SlotPtr tls_slot_; +}; + +class SetResponseCodeFilter : public Http::PassThroughFilter { +public: + SetResponseCodeFilter(std::shared_ptr config) : config_(config) {} + + Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, bool) override { + if (absl::StartsWith(headers.Path()->value().getStringView(), config_->prefix_)) { + decoder_callbacks_->sendLocalReply(static_cast(config_->code_), "", nullptr, + absl::nullopt, ""); + return Http::FilterHeadersStatus::StopIteration; + } + return Http::FilterHeadersStatus::Continue; + } + +private: + const std::shared_ptr config_; +}; + +class SetResponseCodeFilterFactory : public Extensions::HttpFilters::Common::FactoryBase< + test::integration::filters::SetResponseCodeFilterConfig> { +public: + SetResponseCodeFilterFactory() : FactoryBase("set-response-code-filter") {} + +private: + Http::FilterFactoryCb createFilterFactoryFromProtoTyped( + const test::integration::filters::SetResponseCodeFilterConfig& proto_config, + const std::string&, Server::Configuration::FactoryContext& context) override { + auto filter_config = std::make_shared( + proto_config.prefix(), proto_config.code(), context); + return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void { + callbacks.addStreamFilter(std::make_shared(filter_config)); + }; + } +}; + +REGISTER_FACTORY(SetResponseCodeFilterFactory, Server::Configuration::NamedHttpFilterConfigFactory); +} // namespace Envoy diff --git a/test/integration/filters/set_response_code_filter_config.proto b/test/integration/filters/set_response_code_filter_config.proto new file mode 100644 index 0000000000000..f952981ab7a42 --- /dev/null +++ b/test/integration/filters/set_response_code_filter_config.proto @@ -0,0 +1,10 @@ +syntax = "proto3"; + +package test.integration.filters; + +import "validate/validate.proto"; + +message SetResponseCodeFilterConfig { + string prefix = 1; + uint32 code = 2 [(validate.rules).uint32 = {lt: 600 gte: 200}]; +}