From 72d0d96c79f1576a0085f1887829ffcab93fe5ff Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 23 Sep 2024 18:50:40 +0000 Subject: [PATCH] don't directly expose blackboards to filters --- BUILD | 1 + CMakeLists.txt | 2 ++ build_autogenerated.yaml | 4 ++++ .../gcp_authentication_filter.cc | 20 +++++++------------ src/core/lib/channel/promise_based_filter.h | 17 ++++++++++++++-- 5 files changed, 29 insertions(+), 15 deletions(-) diff --git a/BUILD b/BUILD index 2cca36a6afe..0a6f20eea79 100644 --- a/BUILD +++ b/BUILD @@ -2043,6 +2043,7 @@ grpc_cc_library( "//src/core:arena_promise", "//src/core:atomic_utils", "//src/core:bitset", + "//src/core:blackboard", "//src/core:call_destination", "//src/core:call_filters", "//src/core:call_final_info", diff --git a/CMakeLists.txt b/CMakeLists.txt index 312771bd9b5..83f6e3262ae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5277,6 +5277,7 @@ add_library(grpc_authorization_provider src/core/ext/upb-gen/src/proto/grpc/gcp/altscontext.upb_minitable.c src/core/ext/upb-gen/src/proto/grpc/gcp/handshaker.upb_minitable.c src/core/ext/upb-gen/src/proto/grpc/gcp/transport_security_common.upb_minitable.c + src/core/filter/blackboard.cc src/core/handshaker/endpoint_info/endpoint_info_handshaker.cc src/core/handshaker/handshaker.cc src/core/handshaker/handshaker_registry.cc @@ -9066,6 +9067,7 @@ add_executable(call_utils_test src/core/ext/upb-gen/src/proto/grpc/gcp/altscontext.upb_minitable.c src/core/ext/upb-gen/src/proto/grpc/gcp/handshaker.upb_minitable.c src/core/ext/upb-gen/src/proto/grpc/gcp/transport_security_common.upb_minitable.c + src/core/filter/blackboard.cc src/core/handshaker/handshaker_registry.cc src/core/handshaker/proxy_mapper_registry.cc src/core/lib/address_utils/parse_address.cc diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 3cd781c7f5f..e4376f3888c 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -4459,6 +4459,7 @@ libs: - src/core/ext/upb-gen/src/proto/grpc/gcp/handshaker.upb_minitable.h - src/core/ext/upb-gen/src/proto/grpc/gcp/transport_security_common.upb.h - src/core/ext/upb-gen/src/proto/grpc/gcp/transport_security_common.upb_minitable.h + - src/core/filter/blackboard.h - src/core/handshaker/endpoint_info/endpoint_info_handshaker.h - src/core/handshaker/handshaker.h - src/core/handshaker/handshaker_factory.h @@ -4795,6 +4796,7 @@ libs: - src/core/ext/upb-gen/src/proto/grpc/gcp/altscontext.upb_minitable.c - src/core/ext/upb-gen/src/proto/grpc/gcp/handshaker.upb_minitable.c - src/core/ext/upb-gen/src/proto/grpc/gcp/transport_security_common.upb_minitable.c + - src/core/filter/blackboard.cc - src/core/handshaker/endpoint_info/endpoint_info_handshaker.cc - src/core/handshaker/handshaker.cc - src/core/handshaker/handshaker_registry.cc @@ -6779,6 +6781,7 @@ targets: - src/core/ext/upb-gen/src/proto/grpc/gcp/handshaker.upb_minitable.h - src/core/ext/upb-gen/src/proto/grpc/gcp/transport_security_common.upb.h - src/core/ext/upb-gen/src/proto/grpc/gcp/transport_security_common.upb_minitable.h + - src/core/filter/blackboard.h - src/core/handshaker/handshaker_factory.h - src/core/handshaker/handshaker_registry.h - src/core/handshaker/proxy_mapper.h @@ -7084,6 +7087,7 @@ targets: - src/core/ext/upb-gen/src/proto/grpc/gcp/altscontext.upb_minitable.c - src/core/ext/upb-gen/src/proto/grpc/gcp/handshaker.upb_minitable.c - src/core/ext/upb-gen/src/proto/grpc/gcp/transport_security_common.upb_minitable.c + - src/core/filter/blackboard.cc - src/core/handshaker/handshaker_registry.cc - src/core/handshaker/proxy_mapper_registry.cc - src/core/lib/address_utils/parse_address.cc diff --git a/src/core/ext/filters/gcp_authentication/gcp_authentication_filter.cc b/src/core/ext/filters/gcp_authentication/gcp_authentication_filter.cc index 49b6525abfe..7e51b20dca3 100644 --- a/src/core/ext/filters/gcp_authentication/gcp_authentication_filter.cc +++ b/src/core/ext/filters/gcp_authentication/gcp_authentication_filter.cc @@ -176,19 +176,13 @@ GcpAuthenticationFilter::Create(const ChannelArgs& args, "gcp_auth: xds config not found in channel args"); } // Get existing cache or create new one. - RefCountedPtr cache; - if (filter_args.old_blackboard() != nullptr) { - cache = filter_args.old_blackboard()->Get( - filter_config->filter_instance_name); - cache->SetMaxSize(filter_config->cache_size); - } - if (cache == nullptr) { - cache = MakeRefCounted(filter_config->cache_size); - } - if (filter_args.new_blackboard() != nullptr) { - filter_args.new_blackboard()->Set(filter_config->filter_instance_name, - cache); - } + auto cache = filter_args.GetOrCreateState( + filter_config->filter_instance_name, [&]() { + return MakeRefCounted(filter_config->cache_size); + }); + // Make sure size is updated, in case we're reusing a pre-existing + // cache but it has the wrong size. + cache->SetMaxSize(filter_config->cache_size); // Instantiate filter. return std::unique_ptr(new GcpAuthenticationFilter( filter_config, std::move(xds_config), std::move(cache))); diff --git a/src/core/lib/channel/promise_based_filter.h b/src/core/lib/channel/promise_based_filter.h index b67f8f59bd0..6b2ba31ce6b 100644 --- a/src/core/lib/channel/promise_based_filter.h +++ b/src/core/lib/channel/promise_based_filter.h @@ -42,6 +42,7 @@ #include #include +#include "src/core/filter/blackboard.h" #include "src/core/lib/channel/call_finalization.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_fwd.h" @@ -121,8 +122,20 @@ class ChannelFilter { [](const V3Based& v3) { return v3.instance_id; }); } - const Blackboard* old_blackboard() const { return old_blackboard_; } - Blackboard* new_blackboard() const { return new_blackboard_; } + // If a filter state object of type T exists for key from a previous + // filter stack, retains it for the new filter stack we're constructing. + // Otherwise, invokes create_func() to create a new filter state + // object for the new filter stack. Returns the new filter state object. + template + RefCountedPtr GetOrCreateState( + const std::string& key, + absl::FunctionRef()> create_func) { + RefCountedPtr state; + if (old_blackboard_ != nullptr) state = old_blackboard_->Get(key); + if (state == nullptr) state = create_func(); + if (new_blackboard_ != nullptr) new_blackboard_->Set(key, state); + return state; + } private: friend class ChannelFilter;