From 16a3ce51ff7c308c9e8798f1a40b10d172731fab Mon Sep 17 00:00:00 2001 From: Craig Tiller <ctiller@google.com> Date: Wed, 23 Feb 2022 20:50:03 -0800 Subject: [PATCH] Service config parser to core configuration (#28883) * Service config parser to core configuration * x * Automated change: Fix sanity tests * finish * Automated change: Fix sanity tests * oops * fix race * Automated change: Fix sanity tests * back out mutex * refactor * optimize * Automated change: Fix sanity tests * fix * fix * split out interface * review feedback * x * fixes Co-authored-by: ctiller <ctiller@users.noreply.github.com> --- BUILD | 55 ++++- CMakeLists.txt | 4 +- Makefile | 4 +- build_autogenerated.yaml | 6 +- config.m4 | 2 +- config.w32 | 2 +- gRPC-C++.podspec | 2 + gRPC-Core.podspec | 4 +- grpc.gemspec | 3 +- grpc.gyp | 4 +- package.xml | 3 +- .../filters/client_channel/client_channel.cc | 24 +- .../filters/client_channel/client_channel.h | 7 +- .../client_channel/client_channel_plugin.cc | 4 +- .../client_channel/lb_policy/rls/rls.cc | 3 +- .../resolver/dns/c_ares/dns_resolver_ares.cc | 4 +- .../resolver/xds/xds_resolver.cc | 10 +- .../client_channel/resolver_result_parsing.cc | 15 +- .../client_channel/resolver_result_parsing.h | 8 +- .../filters/client_channel/retry_filter.cc | 15 +- .../client_channel/retry_service_config.cc | 12 +- .../client_channel/retry_service_config.h | 8 +- .../service_config_channel_arg_filter.cc | 3 +- .../fault_injection/fault_injection_filter.cc | 18 +- .../fault_injection/service_config_parser.cc | 10 +- .../fault_injection/service_config_parser.h | 10 +- .../message_decompress_filter.cc | 11 +- .../message_size/message_size_filter.cc | 32 ++- .../message_size/message_size_filter.h | 11 +- src/core/ext/filters/rbac/rbac_filter.cc | 10 +- src/core/ext/filters/rbac/rbac_filter.h | 2 + .../rbac/rbac_service_config_parser.cc | 11 +- .../filters/rbac/rbac_service_config_parser.h | 7 +- .../grpc_cronet_plugin_registry.cc | 7 - src/core/ext/xds/xds_server_config_fetcher.cc | 3 +- src/core/lib/config/core_configuration.cc | 3 + src/core/lib/config/core_configuration.h | 11 + src/core/lib/service_config/service_config.h | 57 +---- ...rvice_config.cc => service_config_impl.cc} | 35 +-- .../lib/service_config/service_config_impl.h | 125 ++++++++++ .../service_config/service_config_parser.cc | 56 +++-- .../service_config/service_config_parser.h | 63 ++--- .../plugin_registry/grpc_plugin_registry.cc | 14 +- .../grpc_plugin_registry_extra.cc | 10 +- src/cpp/common/validate_service_config.cc | 6 +- src/python/grpcio/grpc_core_dependencies.py | 2 +- .../rls_lb_config_parser_test.cc | 34 +-- .../client_channel/service_config_test.cc | 228 +++++++++++------- .../rbac/rbac_service_config_parser_test.cc | 32 +-- test/cpp/client/client_channel_stress_test.cc | 3 +- ...channel_with_active_connect_stress_test.cc | 3 +- test/cpp/end2end/client_lb_end2end_test.cc | 3 +- test/cpp/end2end/grpclb_end2end_test.cc | 6 +- test/cpp/end2end/rls_end2end_test.cc | 3 +- .../end2end/service_config_end2end_test.cc | 5 +- test/cpp/naming/resolver_component_test.cc | 11 +- tools/doxygen/Doxyfile.c++.internal | 3 +- tools/doxygen/Doxyfile.core.internal | 3 +- 58 files changed, 649 insertions(+), 401 deletions(-) rename src/core/lib/service_config/{service_config.cc => service_config_impl.cc} (87%) create mode 100644 src/core/lib/service_config/service_config_impl.h diff --git a/BUILD b/BUILD index 855498bd324..f852c38a549 100644 --- a/BUILD +++ b/BUILD @@ -978,6 +978,7 @@ grpc_cc_library( "gpr_base", "grpc_resolver", "handshaker_registry", + "service_config_parser", ], ) @@ -2239,17 +2240,55 @@ grpc_cc_library( grpc_cc_library( name = "grpc_service_config", + hdrs = [ + "src/core/lib/service_config/service_config.h", + "src/core/lib/service_config/service_config_call_data.h", + ], + external_deps = [ + "absl/strings", + ], + language = "c++", + deps = [ + "error", + "gpr_base", + "json", + "service_config_parser", + "slice", + ], +) + +grpc_cc_library( + name = "grpc_service_config_impl", + srcs = [ + "src/core/lib/service_config/service_config_impl.cc", + ], + hdrs = [ + "src/core/lib/service_config/service_config_impl.h", + ], + external_deps = [ + "absl/strings", + ], + language = "c++", + deps = [ + "config", + "error", + "gpr_base", + "grpc_service_config", + "json", + "service_config_parser", + "slice", + ], +) + +grpc_cc_library( + name = "service_config_parser", srcs = [ - "src/core/lib/service_config/service_config.cc", "src/core/lib/service_config/service_config_parser.cc", ], hdrs = [ - "src/core/lib/service_config/service_config.h", - "src/core/lib/service_config/service_config_call_data.h", "src/core/lib/service_config/service_config_parser.h", ], external_deps = [ - "absl/container:inlined_vector", "absl/strings", ], language = "c++", @@ -2257,7 +2296,6 @@ grpc_cc_library( "error", "gpr_base", "json", - "slice", ], ) @@ -2418,6 +2456,7 @@ grpc_cc_library( "grpc_health_upb", "grpc_resolver", "grpc_service_config", + "grpc_service_config_impl", "grpc_trace", "handshaker_registry", "httpcli", @@ -2748,6 +2787,7 @@ grpc_cc_library( "grpc_fake_credentials", "grpc_resolver", "grpc_security_base", + "grpc_service_config_impl", "json", "json_util", "orphanable", @@ -2922,6 +2962,7 @@ grpc_cc_library( "grpc_base", "grpc_server_config_selector", "grpc_server_config_selector_filter", + "grpc_service_config_impl", "grpc_sockaddr", "grpc_xds_channel_stack_modifier", "grpc_xds_client", @@ -3422,6 +3463,7 @@ grpc_cc_library( "grpc_resolver", "grpc_resolver_dns_selection", "grpc_service_config", + "grpc_service_config_impl", "grpc_sockaddr", "iomgr_port", "json", @@ -3518,6 +3560,7 @@ grpc_cc_library( "grpc_client_channel", "grpc_lb_policy_ring_hash", "grpc_resolver", + "grpc_service_config_impl", "grpc_xds_client", ], ) @@ -4630,6 +4673,7 @@ grpc_cc_library( "grpc_codegen", "grpc_health_upb", "grpc_service_config", + "grpc_service_config_impl", "grpc_trace", "grpc_transport_inproc", "ref_counted", @@ -4662,6 +4706,7 @@ grpc_cc_library( "grpc_health_upb", "grpc_insecure_credentials", "grpc_service_config", + "grpc_service_config_impl", "grpc_trace", "grpc_transport_inproc", "grpc_unsecure", diff --git a/CMakeLists.txt b/CMakeLists.txt index 6a71994156b..32c592c8805 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2160,7 +2160,7 @@ add_library(grpc src/core/lib/security/transport/server_auth_filter.cc src/core/lib/security/transport/tsi_error.cc src/core/lib/security/util/json_util.cc - src/core/lib/service_config/service_config.cc + src/core/lib/service_config/service_config_impl.cc src/core/lib/service_config/service_config_parser.cc src/core/lib/slice/b64.cc src/core/lib/slice/percent_encoding.cc @@ -2761,7 +2761,7 @@ add_library(grpc_unsecure src/core/lib/security/transport/server_auth_filter.cc src/core/lib/security/transport/tsi_error.cc src/core/lib/security/util/json_util.cc - src/core/lib/service_config/service_config.cc + src/core/lib/service_config/service_config_impl.cc src/core/lib/service_config/service_config_parser.cc src/core/lib/slice/b64.cc src/core/lib/slice/percent_encoding.cc diff --git a/Makefile b/Makefile index b2e8bae6b43..725d80eb021 100644 --- a/Makefile +++ b/Makefile @@ -1610,7 +1610,7 @@ LIBGRPC_SRC = \ src/core/lib/security/transport/server_auth_filter.cc \ src/core/lib/security/transport/tsi_error.cc \ src/core/lib/security/util/json_util.cc \ - src/core/lib/service_config/service_config.cc \ + src/core/lib/service_config/service_config_impl.cc \ src/core/lib/service_config/service_config_parser.cc \ src/core/lib/slice/b64.cc \ src/core/lib/slice/percent_encoding.cc \ @@ -2060,7 +2060,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/lib/security/transport/server_auth_filter.cc \ src/core/lib/security/transport/tsi_error.cc \ src/core/lib/security/util/json_util.cc \ - src/core/lib/service_config/service_config.cc \ + src/core/lib/service_config/service_config_impl.cc \ src/core/lib/service_config/service_config_parser.cc \ src/core/lib/slice/b64.cc \ src/core/lib/slice/percent_encoding.cc \ diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index cbdeaadea87..cda30eeb1a1 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -1005,6 +1005,7 @@ libs: - src/core/lib/security/util/json_util.h - src/core/lib/service_config/service_config.h - src/core/lib/service_config/service_config_call_data.h + - src/core/lib/service_config/service_config_impl.h - src/core/lib/service_config/service_config_parser.h - src/core/lib/slice/b64.h - src/core/lib/slice/percent_encoding.h @@ -1659,7 +1660,7 @@ libs: - src/core/lib/security/transport/server_auth_filter.cc - src/core/lib/security/transport/tsi_error.cc - src/core/lib/security/util/json_util.cc - - src/core/lib/service_config/service_config.cc + - src/core/lib/service_config/service_config_impl.cc - src/core/lib/service_config/service_config_parser.cc - src/core/lib/slice/b64.cc - src/core/lib/slice/percent_encoding.cc @@ -2138,6 +2139,7 @@ libs: - src/core/lib/security/util/json_util.h - src/core/lib/service_config/service_config.h - src/core/lib/service_config/service_config_call_data.h + - src/core/lib/service_config/service_config_impl.h - src/core/lib/service_config/service_config_parser.h - src/core/lib/slice/b64.h - src/core/lib/slice/percent_encoding.h @@ -2438,7 +2440,7 @@ libs: - src/core/lib/security/transport/server_auth_filter.cc - src/core/lib/security/transport/tsi_error.cc - src/core/lib/security/util/json_util.cc - - src/core/lib/service_config/service_config.cc + - src/core/lib/service_config/service_config_impl.cc - src/core/lib/service_config/service_config_parser.cc - src/core/lib/slice/b64.cc - src/core/lib/slice/percent_encoding.cc diff --git a/config.m4 b/config.m4 index 216ccee9380..4f9adf1581a 100644 --- a/config.m4 +++ b/config.m4 @@ -672,7 +672,7 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/security/transport/server_auth_filter.cc \ src/core/lib/security/transport/tsi_error.cc \ src/core/lib/security/util/json_util.cc \ - src/core/lib/service_config/service_config.cc \ + src/core/lib/service_config/service_config_impl.cc \ src/core/lib/service_config/service_config_parser.cc \ src/core/lib/slice/b64.cc \ src/core/lib/slice/percent_encoding.cc \ diff --git a/config.w32 b/config.w32 index 0f7596831b5..b6316d6005d 100644 --- a/config.w32 +++ b/config.w32 @@ -638,7 +638,7 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\security\\transport\\server_auth_filter.cc " + "src\\core\\lib\\security\\transport\\tsi_error.cc " + "src\\core\\lib\\security\\util\\json_util.cc " + - "src\\core\\lib\\service_config\\service_config.cc " + + "src\\core\\lib\\service_config\\service_config_impl.cc " + "src\\core\\lib\\service_config\\service_config_parser.cc " + "src\\core\\lib\\slice\\b64.cc " + "src\\core\\lib\\slice\\percent_encoding.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index b087a4e84c3..19f192ec9c3 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -859,6 +859,7 @@ Pod::Spec.new do |s| 'src/core/lib/security/util/json_util.h', 'src/core/lib/service_config/service_config.h', 'src/core/lib/service_config/service_config_call_data.h', + 'src/core/lib/service_config/service_config_impl.h', 'src/core/lib/service_config/service_config_parser.h', 'src/core/lib/slice/b64.h', 'src/core/lib/slice/percent_encoding.h', @@ -1655,6 +1656,7 @@ Pod::Spec.new do |s| 'src/core/lib/security/util/json_util.h', 'src/core/lib/service_config/service_config.h', 'src/core/lib/service_config/service_config_call_data.h', + 'src/core/lib/service_config/service_config_impl.h', 'src/core/lib/service_config/service_config_parser.h', 'src/core/lib/slice/b64.h', 'src/core/lib/slice/percent_encoding.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 53ab4548cd7..9175c3e15f4 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1428,9 +1428,10 @@ Pod::Spec.new do |s| 'src/core/lib/security/transport/tsi_error.h', 'src/core/lib/security/util/json_util.cc', 'src/core/lib/security/util/json_util.h', - 'src/core/lib/service_config/service_config.cc', 'src/core/lib/service_config/service_config.h', 'src/core/lib/service_config/service_config_call_data.h', + 'src/core/lib/service_config/service_config_impl.cc', + 'src/core/lib/service_config/service_config_impl.h', 'src/core/lib/service_config/service_config_parser.cc', 'src/core/lib/service_config/service_config_parser.h', 'src/core/lib/slice/b64.cc', @@ -2248,6 +2249,7 @@ Pod::Spec.new do |s| 'src/core/lib/security/util/json_util.h', 'src/core/lib/service_config/service_config.h', 'src/core/lib/service_config/service_config_call_data.h', + 'src/core/lib/service_config/service_config_impl.h', 'src/core/lib/service_config/service_config_parser.h', 'src/core/lib/slice/b64.h', 'src/core/lib/slice/percent_encoding.h', diff --git a/grpc.gemspec b/grpc.gemspec index 2e4e256d915..9367f39fd38 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1347,9 +1347,10 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/security/transport/tsi_error.h ) s.files += %w( src/core/lib/security/util/json_util.cc ) s.files += %w( src/core/lib/security/util/json_util.h ) - s.files += %w( src/core/lib/service_config/service_config.cc ) s.files += %w( src/core/lib/service_config/service_config.h ) s.files += %w( src/core/lib/service_config/service_config_call_data.h ) + s.files += %w( src/core/lib/service_config/service_config_impl.cc ) + s.files += %w( src/core/lib/service_config/service_config_impl.h ) s.files += %w( src/core/lib/service_config/service_config_parser.cc ) s.files += %w( src/core/lib/service_config/service_config_parser.h ) s.files += %w( src/core/lib/slice/b64.cc ) diff --git a/grpc.gyp b/grpc.gyp index 57aff9ab2b5..5dcae0a9a93 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -1064,7 +1064,7 @@ 'src/core/lib/security/transport/server_auth_filter.cc', 'src/core/lib/security/transport/tsi_error.cc', 'src/core/lib/security/util/json_util.cc', - 'src/core/lib/service_config/service_config.cc', + 'src/core/lib/service_config/service_config_impl.cc', 'src/core/lib/service_config/service_config_parser.cc', 'src/core/lib/slice/b64.cc', 'src/core/lib/slice/percent_encoding.cc', @@ -1485,7 +1485,7 @@ 'src/core/lib/security/transport/server_auth_filter.cc', 'src/core/lib/security/transport/tsi_error.cc', 'src/core/lib/security/util/json_util.cc', - 'src/core/lib/service_config/service_config.cc', + 'src/core/lib/service_config/service_config_impl.cc', 'src/core/lib/service_config/service_config_parser.cc', 'src/core/lib/slice/b64.cc', 'src/core/lib/slice/percent_encoding.cc', diff --git a/package.xml b/package.xml index f9126855a2f..e0a9c081bcd 100644 --- a/package.xml +++ b/package.xml @@ -1327,9 +1327,10 @@ <file baseinstalldir="/" name="src/core/lib/security/transport/tsi_error.h" role="src" /> <file baseinstalldir="/" name="src/core/lib/security/util/json_util.cc" role="src" /> <file baseinstalldir="/" name="src/core/lib/security/util/json_util.h" role="src" /> - <file baseinstalldir="/" name="src/core/lib/service_config/service_config.cc" role="src" /> <file baseinstalldir="/" name="src/core/lib/service_config/service_config.h" role="src" /> <file baseinstalldir="/" name="src/core/lib/service_config/service_config_call_data.h" role="src" /> + <file baseinstalldir="/" name="src/core/lib/service_config/service_config_impl.cc" role="src" /> + <file baseinstalldir="/" name="src/core/lib/service_config/service_config_impl.h" role="src" /> <file baseinstalldir="/" name="src/core/lib/service_config/service_config_parser.cc" role="src" /> <file baseinstalldir="/" name="src/core/lib/service_config/service_config_parser.h" role="src" /> <file baseinstalldir="/" name="src/core/lib/slice/b64.cc" role="src" /> diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 92784da947b..56761074870 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -63,8 +63,8 @@ #include "src/core/lib/iomgr/work_serializer.h" #include "src/core/lib/profiling/timers.h" #include "src/core/lib/resolver/resolver_registry.h" -#include "src/core/lib/service_config/service_config.h" #include "src/core/lib/service_config/service_config_call_data.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" #include "src/core/lib/surface/channel.h" @@ -1036,6 +1036,8 @@ ClientChannel::ClientChannel(grpc_channel_element_args* args, ClientChannelFactory::GetFromChannelArgs(args->channel_args)), channelz_node_(GetChannelzNode(args->channel_args)), interested_parties_(grpc_pollset_set_create()), + service_config_parser_index_( + internal::ClientChannelServiceConfigParser::ParserIndex()), work_serializer_(std::make_shared<WorkSerializer>()), state_tracker_("client_channel", GRPC_CHANNEL_IDLE), subchannel_pool_(GetSubchannelPool(args->channel_args)) { @@ -1058,7 +1060,7 @@ ClientChannel::ClientChannel(grpc_channel_element_args* args, if (service_config_json == nullptr) service_config_json = "{}"; *error = GRPC_ERROR_NONE; default_service_config_ = - ServiceConfig::Create(args->channel_args, service_config_json, error); + ServiceConfigImpl::Create(args->channel_args, service_config_json, error); if (*error != GRPC_ERROR_NONE) { default_service_config_.reset(); return; @@ -1278,7 +1280,7 @@ void ClientChannel::OnResolverResultChangedLocked(Resolver::Result result) { const internal::ClientChannelGlobalParsedConfig* parsed_service_config = static_cast<const internal::ClientChannelGlobalParsedConfig*>( service_config->GetGlobalParsedConfig( - internal::ClientChannelServiceConfigParser::ParserIndex())); + service_config_parser_index_)); // Choose LB policy config. RefCountedPtr<LoadBalancingPolicy::Config> lb_policy_config = ChooseLbPolicy(result, parsed_service_config); @@ -1438,21 +1440,19 @@ void ClientChannel::RemoveResolverQueuedCall(ResolverQueuedCall* to_remove, void ClientChannel::UpdateServiceConfigInControlPlaneLocked( RefCountedPtr<ServiceConfig> service_config, - RefCountedPtr<ConfigSelector> config_selector, const char* lb_policy_name) { - UniquePtr<char> service_config_json( - gpr_strdup(service_config->json_string().c_str())); + RefCountedPtr<ConfigSelector> config_selector, std::string lb_policy_name) { + std::string service_config_json(service_config->json_string()); if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_routing_trace)) { gpr_log(GPR_INFO, "chand=%p: resolver returned updated service config: \"%s\"", this, - service_config_json.get()); + service_config_json.c_str()); } // Save service config. saved_service_config_ = std::move(service_config); // Swap out the data used by GetChannelInfo(). - UniquePtr<char> lb_policy_name_owned(gpr_strdup(lb_policy_name)); { MutexLock lock(&info_mu_); - info_lb_policy_name_ = std::move(lb_policy_name_owned); + info_lb_policy_name_ = std::move(lb_policy_name); info_service_config_json_ = std::move(service_config_json); } // Save config selector. @@ -1784,11 +1784,11 @@ void ClientChannel::GetChannelInfo(grpc_channel_element* elem, ClientChannel* chand = static_cast<ClientChannel*>(elem->channel_data); MutexLock lock(&chand->info_mu_); if (info->lb_policy_name != nullptr) { - *info->lb_policy_name = gpr_strdup(chand->info_lb_policy_name_.get()); + *info->lb_policy_name = gpr_strdup(chand->info_lb_policy_name_.c_str()); } if (info->service_config_json != nullptr) { *info->service_config_json = - gpr_strdup(chand->info_service_config_json_.get()); + gpr_strdup(chand->info_service_config_json_.c_str()); } } @@ -2223,7 +2223,7 @@ grpc_error_handle ClientChannel::CallData::ApplyServiceConfigToCallLocked( // Apply our own method params to the call. auto* method_params = static_cast<ClientChannelMethodParsedConfig*>( service_config_call_data->GetMethodParsedConfig( - internal::ClientChannelServiceConfigParser::ParserIndex())); + chand->service_config_parser_index_)); if (method_params != nullptr) { // If the deadline from the service config is shorter than the one // from the client API, reset the deadline timer. diff --git a/src/core/ext/filters/client_channel/client_channel.h b/src/core/ext/filters/client_channel/client_channel.h index e87fa8d408b..05929279386 100644 --- a/src/core/ext/filters/client_channel/client_channel.h +++ b/src/core/ext/filters/client_channel/client_channel.h @@ -235,7 +235,7 @@ class ClientChannel { void UpdateServiceConfigInControlPlaneLocked( RefCountedPtr<ServiceConfig> service_config, - RefCountedPtr<ConfigSelector> config_selector, const char* lb_policy_name) + RefCountedPtr<ConfigSelector> config_selector, std::string lb_policy_name) ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_); void UpdateServiceConfigInDataPlaneLocked() @@ -279,6 +279,7 @@ class ClientChannel { std::string default_authority_; channelz::ChannelNode* channelz_node_; grpc_pollset_set* interested_parties_; + const size_t service_config_parser_index_; // // Fields related to name resolution. Guarded by resolution_mu_. @@ -339,8 +340,8 @@ class ClientChannel { // synchronously via get_channel_info(). // Mutex info_mu_; - UniquePtr<char> info_lb_policy_name_ ABSL_GUARDED_BY(info_mu_); - UniquePtr<char> info_service_config_json_ ABSL_GUARDED_BY(info_mu_); + std::string info_lb_policy_name_ ABSL_GUARDED_BY(info_mu_); + std::string info_service_config_json_ ABSL_GUARDED_BY(info_mu_); // // Fields guarded by a mutex, since they need to be accessed diff --git a/src/core/ext/filters/client_channel/client_channel_plugin.cc b/src/core/ext/filters/client_channel/client_channel_plugin.cc index 0f3fa830fc2..6fbddcf378f 100644 --- a/src/core/ext/filters/client_channel/client_channel_plugin.cc +++ b/src/core/ext/filters/client_channel/client_channel_plugin.cc @@ -39,8 +39,6 @@ #include "src/core/lib/resolver/resolver_registry.h" void grpc_client_channel_init(void) { - grpc_core::internal::ClientChannelServiceConfigParser::Register(); - grpc_core::internal::RetryServiceConfigParser::Register(); grpc_core::LoadBalancingPolicyRegistry::Builder::InitRegistry(); grpc_core::internal::ServerRetryThrottleMap::Init(); grpc_core::ProxyMapperRegistry::Init(); @@ -60,6 +58,8 @@ namespace grpc_core { void BuildClientChannelConfiguration(CoreConfiguration::Builder* builder) { RegisterHttpConnectHandshaker(builder); + internal::ClientChannelServiceConfigParser::Register(builder); + internal::RetryServiceConfigParser::Register(builder); builder->channel_init()->RegisterStage( GRPC_CLIENT_CHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY, [](ChannelStackBuilder* builder) { diff --git a/src/core/ext/filters/client_channel/lb_policy/rls/rls.cc b/src/core/ext/filters/client_channel/lb_policy/rls/rls.cc index abfd560a863..1c53df3359b 100644 --- a/src/core/ext/filters/client_channel/lb_policy/rls/rls.cc +++ b/src/core/ext/filters/client_channel/lb_policy/rls/rls.cc @@ -69,6 +69,7 @@ #include "src/core/lib/resolver/resolver_registry.h" #include "src/core/lib/security/credentials/credentials.h" #include "src/core/lib/security/credentials/fake/fake_credentials.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/core/lib/surface/call.h" #include "src/core/lib/surface/channel.h" #include "src/core/lib/transport/connectivity_state.h" @@ -2478,7 +2479,7 @@ class RlsLbFactory : public LoadBalancingPolicyFactory { Json rls_channel_service_config_json( *rls_channel_service_config_json_obj); rls_channel_service_config = rls_channel_service_config_json.Dump(); - auto service_config = MakeRefCounted<ServiceConfig>( + auto service_config = MakeRefCounted<ServiceConfigImpl>( /*args=*/nullptr, rls_channel_service_config, std::move(rls_channel_service_config_json), &child_error); if (child_error != GRPC_ERROR_NONE) { diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index 14134302fce..3ecbfcfe4de 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -48,7 +48,7 @@ #include "src/core/lib/json/json.h" #include "src/core/lib/resolver/resolver_registry.h" #include "src/core/lib/resolver/server_address.h" -#include "src/core/lib/service_config/service_config.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/core/lib/transport/error_utils.h" #define GRPC_DNS_INITIAL_CONNECT_BACKOFF_SECONDS 1 @@ -346,7 +346,7 @@ void AresClientChannelDNSResolver::OnResolvedLocked(grpc_error_handle error) { !service_config_string.empty()) { GRPC_CARES_TRACE_LOG("resolver:%p selected service config choice: %s", this, service_config_string.c_str()); - service_config = ServiceConfig::Create( + service_config = ServiceConfigImpl::Create( channel_args_, service_config_string, &service_config_error); } if (service_config_error != GRPC_ERROR_NONE) { diff --git a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc index 0c3f82be428..a4e57646545 100644 --- a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc @@ -39,6 +39,7 @@ #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/resolver/resolver_registry.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/core/lib/transport/error_utils.h" #include "src/core/lib/transport/timeout_encoding.h" @@ -542,7 +543,8 @@ grpc_error_handle XdsResolver::XdsConfigSelector::CreateMethodConfig( absl::StrJoin(fields, ",\n"), "\n } ]\n" "}"); - *method_config = ServiceConfig::Create(result.args, json.c_str(), &error); + *method_config = + ServiceConfigImpl::Create(result.args, json.c_str(), &error); } grpc_channel_args_destroy(result.args); return error; @@ -895,7 +897,7 @@ void XdsResolver::OnResourceDoesNotExist() { current_virtual_host_.routes.clear(); Result result; grpc_error_handle error = GRPC_ERROR_NONE; - result.service_config = ServiceConfig::Create(args_, "{}", &error); + result.service_config = ServiceConfigImpl::Create(args_, "{}", &error); GPR_ASSERT(*result.service_config != nullptr); result.args = grpc_channel_args_copy(args_); result_handler_->ReportResult(std::move(result)); @@ -930,7 +932,7 @@ XdsResolver::CreateServiceConfig() { std::string json = absl::StrJoin(config_parts, ""); grpc_error_handle error = GRPC_ERROR_NONE; absl::StatusOr<RefCountedPtr<ServiceConfig>> result = - ServiceConfig::Create(args_, json.c_str(), &error); + ServiceConfigImpl::Create(args_, json.c_str(), &error); if (error != GRPC_ERROR_NONE) { result = grpc_error_to_absl_status(error); GRPC_ERROR_UNREF(error); @@ -954,7 +956,7 @@ void XdsResolver::GenerateResult() { if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_resolver_trace)) { gpr_log(GPR_INFO, "[xds_resolver %p] generated service config: %s", this, result.service_config.ok() - ? (*result.service_config)->json_string().c_str() + ? std::string((*result.service_config)->json_string()).c_str() : result.service_config.status().ToString().c_str()); } grpc_arg new_args[] = { diff --git a/src/core/ext/filters/client_channel/resolver_result_parsing.cc b/src/core/ext/filters/client_channel/resolver_result_parsing.cc index 17b734fad34..64a468f844f 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.cc +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.cc @@ -45,18 +45,15 @@ namespace grpc_core { namespace internal { -namespace { -size_t g_client_channel_service_config_parser_index; -} - size_t ClientChannelServiceConfigParser::ParserIndex() { - return g_client_channel_service_config_parser_index; + return CoreConfiguration::Get().service_config_parser().GetParserIndex( + parser_name()); } -void ClientChannelServiceConfigParser::Register() { - g_client_channel_service_config_parser_index = - ServiceConfigParser::RegisterParser( - absl::make_unique<ClientChannelServiceConfigParser>()); +void ClientChannelServiceConfigParser::Register( + CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( + absl::make_unique<ClientChannelServiceConfigParser>()); } namespace { diff --git a/src/core/ext/filters/client_channel/resolver_result_parsing.h b/src/core/ext/filters/client_channel/resolver_result_parsing.h index 56979cb1531..fea59971ecd 100644 --- a/src/core/ext/filters/client_channel/resolver_result_parsing.h +++ b/src/core/ext/filters/client_channel/resolver_result_parsing.h @@ -24,6 +24,7 @@ #include "src/core/ext/filters/client_channel/lb_policy.h" #include "src/core/ext/filters/client_channel/lb_policy_factory.h" #include "src/core/lib/channel/status_util.h" +#include "src/core/lib/config/core_configuration.h" #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/iomgr/exec_ctx.h" // for grpc_millis @@ -81,6 +82,8 @@ class ClientChannelMethodParsedConfig class ClientChannelServiceConfigParser : public ServiceConfigParser::Parser { public: + absl::string_view name() const override { return parser_name(); } + std::unique_ptr<ServiceConfigParser::ParsedConfig> ParseGlobalParams( const grpc_channel_args* /*args*/, const Json& json, grpc_error_handle* error) override; @@ -90,7 +93,10 @@ class ClientChannelServiceConfigParser : public ServiceConfigParser::Parser { grpc_error_handle* error) override; static size_t ParserIndex(); - static void Register(); + static void Register(CoreConfiguration::Builder* builder); + + private: + static absl::string_view parser_name() { return "client_channel"; } }; } // namespace internal diff --git a/src/core/ext/filters/client_channel/retry_filter.cc b/src/core/ext/filters/client_channel/retry_filter.cc index 4fc34f62c08..d320a3a51af 100644 --- a/src/core/ext/filters/client_channel/retry_filter.cc +++ b/src/core/ext/filters/client_channel/retry_filter.cc @@ -145,7 +145,9 @@ class RetryFilter { RetryFilter(const grpc_channel_args* args, grpc_error_handle* error) : client_channel_(grpc_channel_args_find_pointer<ClientChannel>( args, GRPC_ARG_CLIENT_CHANNEL)), - per_rpc_retry_buffer_size_(GetMaxPerRpcRetryBufferSize(args)) { + per_rpc_retry_buffer_size_(GetMaxPerRpcRetryBufferSize(args)), + service_config_parser_index_( + internal::RetryServiceConfigParser::ParserIndex()) { // Get retry throttling parameters from service config. auto* service_config = grpc_channel_args_find_pointer<ServiceConfig>( args, GRPC_ARG_SERVICE_CONFIG_OBJ); @@ -175,9 +177,13 @@ class RetryFilter { server_name, config->max_milli_tokens(), config->milli_token_ratio()); } + const RetryMethodConfig* GetRetryPolicy( + const grpc_call_context_element* context); + ClientChannel* client_channel_; size_t per_rpc_retry_buffer_size_; RefCountedPtr<ServerRetryThrottleData> retry_throttle_data_; + const size_t service_config_parser_index_; }; // @@ -2049,22 +2055,21 @@ void RetryFilter::CallData::SetPollent(grpc_call_element* elem, // CallData implementation // -const RetryMethodConfig* GetRetryPolicy( +const RetryMethodConfig* RetryFilter::GetRetryPolicy( const grpc_call_context_element* context) { if (context == nullptr) return nullptr; auto* svc_cfg_call_data = static_cast<ServiceConfigCallData*>( context[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA].value); if (svc_cfg_call_data == nullptr) return nullptr; return static_cast<const RetryMethodConfig*>( - svc_cfg_call_data->GetMethodParsedConfig( - RetryServiceConfigParser::ParserIndex())); + svc_cfg_call_data->GetMethodParsedConfig(service_config_parser_index_)); } RetryFilter::CallData::CallData(RetryFilter* chand, const grpc_call_element_args& args) : chand_(chand), retry_throttle_data_(chand->retry_throttle_data_), - retry_policy_(GetRetryPolicy(args.context)), + retry_policy_(chand->GetRetryPolicy(args.context)), retry_backoff_( BackOff::Options() .set_initial_backoff(retry_policy_ == nullptr diff --git a/src/core/ext/filters/client_channel/retry_service_config.cc b/src/core/ext/filters/client_channel/retry_service_config.cc index d5bd37fa994..0a670cc3fd6 100644 --- a/src/core/ext/filters/client_channel/retry_service_config.cc +++ b/src/core/ext/filters/client_channel/retry_service_config.cc @@ -33,6 +33,7 @@ #include "src/core/ext/filters/client_channel/lb_policy_registry.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/status_util.h" +#include "src/core/lib/config/core_configuration.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/json/json_util.h" @@ -45,16 +46,13 @@ namespace grpc_core { namespace internal { -namespace { -size_t g_retry_service_config_parser_index; -} - size_t RetryServiceConfigParser::ParserIndex() { - return g_retry_service_config_parser_index; + return CoreConfiguration::Get().service_config_parser().GetParserIndex( + parser_name()); } -void RetryServiceConfigParser::Register() { - g_retry_service_config_parser_index = ServiceConfigParser::RegisterParser( +void RetryServiceConfigParser::Register(CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( absl::make_unique<RetryServiceConfigParser>()); } diff --git a/src/core/ext/filters/client_channel/retry_service_config.h b/src/core/ext/filters/client_channel/retry_service_config.h index e73f09463f9..d9a8d5b114a 100644 --- a/src/core/ext/filters/client_channel/retry_service_config.h +++ b/src/core/ext/filters/client_channel/retry_service_config.h @@ -23,6 +23,7 @@ #include "src/core/ext/filters/client_channel/retry_throttle.h" #include "src/core/lib/channel/status_util.h" +#include "src/core/lib/config/core_configuration.h" #include "src/core/lib/iomgr/exec_ctx.h" // for grpc_millis #include "src/core/lib/service_config/service_config_parser.h" @@ -78,6 +79,8 @@ class RetryMethodConfig : public ServiceConfigParser::ParsedConfig { class RetryServiceConfigParser : public ServiceConfigParser::Parser { public: + absl::string_view name() const override { return parser_name(); } + std::unique_ptr<ServiceConfigParser::ParsedConfig> ParseGlobalParams( const grpc_channel_args* /*args*/, const Json& json, grpc_error_handle* error) override; @@ -87,7 +90,10 @@ class RetryServiceConfigParser : public ServiceConfigParser::Parser { grpc_error_handle* error) override; static size_t ParserIndex(); - static void Register(); + static void Register(CoreConfiguration::Builder* builder); + + private: + static absl::string_view parser_name() { return "retry"; } }; } // namespace internal diff --git a/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc b/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc index ebd7d198fec..34d3382d5c8 100644 --- a/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc +++ b/src/core/ext/filters/client_channel/service_config_channel_arg_filter.cc @@ -24,6 +24,7 @@ #include "src/core/lib/channel/channel_stack_builder.h" #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/service_config/service_config_call_data.h" +#include "src/core/lib/service_config/service_config_impl.h" namespace grpc_core { @@ -37,7 +38,7 @@ class ServiceConfigChannelArgChannelData { args->channel_args, GRPC_ARG_SERVICE_CONFIG); if (service_config_str != nullptr) { grpc_error_handle service_config_error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( args->channel_args, service_config_str, &service_config_error); if (service_config_error == GRPC_ERROR_NONE) { service_config_ = std::move(service_config); diff --git a/src/core/ext/filters/fault_injection/fault_injection_filter.cc b/src/core/ext/filters/fault_injection/fault_injection_filter.cc index fce1a4fc9ef..3c05d955ba4 100644 --- a/src/core/ext/filters/fault_injection/fault_injection_filter.cc +++ b/src/core/ext/filters/fault_injection/fault_injection_filter.cc @@ -69,6 +69,9 @@ class ChannelData { static void Destroy(grpc_channel_element* elem); int index() const { return index_; } + size_t service_config_parser_index() const { + return service_config_parser_index_; + } private: ChannelData(grpc_channel_element* elem, grpc_channel_element_args* args); @@ -76,6 +79,7 @@ class ChannelData { // The relative index of instances of the same filter. int index_; + const size_t service_config_parser_index_; }; class CallData { @@ -170,8 +174,10 @@ void ChannelData::Destroy(grpc_channel_element* elem) { ChannelData::ChannelData(grpc_channel_element* elem, grpc_channel_element_args* args) - : index_(grpc_channel_stack_filter_instance_number(args->channel_stack, - elem)) {} + : index_( + grpc_channel_stack_filter_instance_number(args->channel_stack, elem)), + service_config_parser_index_( + FaultInjectionServiceConfigParser::ParserIndex()) {} // CallData::ResumeBatchCanceller @@ -294,7 +300,7 @@ CallData::CallData(grpc_call_element* elem, const grpc_call_element_args* args) args->context[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA].value); auto* method_params = static_cast<FaultInjectionMethodParsedConfig*>( service_config_call_data->GetMethodParsedConfig( - FaultInjectionServiceConfigParser::ParserIndex())); + chand->service_config_parser_index())); if (method_params != nullptr) { fi_policy_ = method_params->fault_injection_policy(chand->index()); } @@ -484,10 +490,8 @@ extern const grpc_channel_filter FaultInjectionFilterVtable = { "fault_injection_filter", }; -void FaultInjectionFilterInit(void) { - FaultInjectionServiceConfigParser::Register(); +void FaultInjectionFilterRegister(CoreConfiguration::Builder* builder) { + FaultInjectionServiceConfigParser::Register(builder); } -void FaultInjectionFilterShutdown(void) {} - } // namespace grpc_core diff --git a/src/core/ext/filters/fault_injection/service_config_parser.cc b/src/core/ext/filters/fault_injection/service_config_parser.cc index bfdbb6f17ae..df5d302697d 100644 --- a/src/core/ext/filters/fault_injection/service_config_parser.cc +++ b/src/core/ext/filters/fault_injection/service_config_parser.cc @@ -33,8 +33,6 @@ namespace grpc_core { namespace { -size_t g_fault_injection_parser_index; - std::vector<FaultInjectionMethodParsedConfig::FaultInjectionPolicy> ParseFaultInjectionPolicy(const Json::Array& policies_json_array, std::vector<grpc_error_handle>* error_list) { @@ -167,13 +165,15 @@ FaultInjectionServiceConfigParser::ParsePerMethodParams( std::move(fault_injection_policies)); } -void FaultInjectionServiceConfigParser::Register() { - g_fault_injection_parser_index = ServiceConfigParser::RegisterParser( +void FaultInjectionServiceConfigParser::Register( + CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( absl::make_unique<FaultInjectionServiceConfigParser>()); } size_t FaultInjectionServiceConfigParser::ParserIndex() { - return g_fault_injection_parser_index; + return CoreConfiguration::Get().service_config_parser().GetParserIndex( + parser_name()); } } // namespace grpc_core diff --git a/src/core/ext/filters/fault_injection/service_config_parser.h b/src/core/ext/filters/fault_injection/service_config_parser.h index af60237ef10..2a01fba7532 100644 --- a/src/core/ext/filters/fault_injection/service_config_parser.h +++ b/src/core/ext/filters/fault_injection/service_config_parser.h @@ -21,6 +21,7 @@ #include <vector> +#include "src/core/lib/config/core_configuration.h" #include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/service_config/service_config_parser.h" @@ -68,8 +69,10 @@ class FaultInjectionMethodParsedConfig std::vector<FaultInjectionPolicy> fault_injection_policies_; }; -class FaultInjectionServiceConfigParser : public ServiceConfigParser::Parser { +class FaultInjectionServiceConfigParser final + : public ServiceConfigParser::Parser { public: + absl::string_view name() const override { return parser_name(); } // Parses the per-method service config for fault injection filter. std::unique_ptr<ServiceConfigParser::ParsedConfig> ParsePerMethodParams( const grpc_channel_args* args, const Json& json, @@ -77,7 +80,10 @@ class FaultInjectionServiceConfigParser : public ServiceConfigParser::Parser { // Returns the parser index for FaultInjectionServiceConfigParser. static size_t ParserIndex(); // Registers FaultInjectionServiceConfigParser to ServiceConfigParser. - static void Register(); + static void Register(CoreConfiguration::Builder* builder); + + private: + static absl::string_view parser_name() { return "fault_injection"; } }; } // namespace grpc_core diff --git a/src/core/ext/filters/http/message_compress/message_decompress_filter.cc b/src/core/ext/filters/http/message_compress/message_decompress_filter.cc index 36a54335a1e..aab2dbc39a4 100644 --- a/src/core/ext/filters/http/message_compress/message_decompress_filter.cc +++ b/src/core/ext/filters/http/message_compress/message_decompress_filter.cc @@ -45,12 +45,18 @@ namespace { class ChannelData { public: explicit ChannelData(const grpc_channel_element_args* args) - : max_recv_size_(GetMaxRecvSizeFromChannelArgs(args->channel_args)) {} + : max_recv_size_(GetMaxRecvSizeFromChannelArgs(args->channel_args)), + message_size_service_config_parser_index_( + MessageSizeParser::ParserIndex()) {} int max_recv_size() const { return max_recv_size_; } + size_t message_size_service_config_parser_index() const { + return message_size_service_config_parser_index_; + } private: int max_recv_size_; + const size_t message_size_service_config_parser_index_; }; class CallData { @@ -73,7 +79,8 @@ class CallData { OnRecvTrailingMetadataReady, this, grpc_schedule_on_exec_ctx); const MessageSizeParsedConfig* limits = - MessageSizeParsedConfig::GetFromCallContext(args.context); + MessageSizeParsedConfig::GetFromCallContext( + args.context, chand->message_size_service_config_parser_index()); if (limits != nullptr && limits->limits().max_recv_size >= 0 && (limits->limits().max_recv_size < max_recv_message_length_ || max_recv_message_length_ < 0)) { diff --git a/src/core/ext/filters/message_size/message_size_filter.cc b/src/core/ext/filters/message_size/message_size_filter.cc index a672f150e88..7be43729aa2 100644 --- a/src/core/ext/filters/message_size/message_size_filter.cc +++ b/src/core/ext/filters/message_size/message_size_filter.cc @@ -27,6 +27,7 @@ #include <grpc/support/alloc.h> #include <grpc/support/log.h> +#include "src/core/ext/filters/message_size/message_size_filter.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_stack_builder.h" #include "src/core/lib/config/core_configuration.h" @@ -42,23 +43,19 @@ static void recv_trailing_metadata_ready(void* user_data, namespace grpc_core { -namespace { -size_t g_message_size_parser_index; -} // namespace - // // MessageSizeParsedConfig // const MessageSizeParsedConfig* MessageSizeParsedConfig::GetFromCallContext( - const grpc_call_context_element* context) { + const grpc_call_context_element* context, + size_t service_config_parser_index) { if (context == nullptr) return nullptr; auto* svc_cfg_call_data = static_cast<ServiceConfigCallData*>( context[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA].value); if (svc_cfg_call_data == nullptr) return nullptr; return static_cast<const MessageSizeParsedConfig*>( - svc_cfg_call_data->GetMethodParsedConfig( - MessageSizeParser::ParserIndex())); + svc_cfg_call_data->GetMethodParsedConfig(service_config_parser_index)); } // @@ -113,12 +110,15 @@ MessageSizeParser::ParsePerMethodParams(const grpc_channel_args* /*args*/, max_response_message_bytes); } -void MessageSizeParser::Register() { - g_message_size_parser_index = ServiceConfigParser::RegisterParser( +void MessageSizeParser::Register(CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( absl::make_unique<MessageSizeParser>()); } -size_t MessageSizeParser::ParserIndex() { return g_message_size_parser_index; } +size_t MessageSizeParser::ParserIndex() { + return CoreConfiguration::Get().service_config_parser().GetParserIndex( + parser_name()); +} int GetMaxRecvSizeFromChannelArgs(const grpc_channel_args* args) { if (grpc_channel_args_want_minimal_stack(args)) return -1; @@ -139,6 +139,8 @@ int GetMaxSendSizeFromChannelArgs(const grpc_channel_args* args) { namespace { struct channel_data { grpc_core::MessageSizeParsedConfig::message_size_limits limits; + const size_t service_config_parser_index{ + grpc_core::MessageSizeParser::ParserIndex()}; }; struct call_data { @@ -155,7 +157,8 @@ struct call_data { // apply the max request size to the send limit and the max response // size to the receive limit. const grpc_core::MessageSizeParsedConfig* limits = - grpc_core::MessageSizeParsedConfig::GetFromCallContext(args.context); + grpc_core::MessageSizeParsedConfig::GetFromCallContext( + args.context, chand.service_config_parser_index); if (limits != nullptr) { if (limits->limits().max_send_size >= 0 && (limits->limits().max_send_size < this->limits.max_send_size || @@ -375,14 +378,9 @@ static bool maybe_add_message_size_filter( return true; } -void grpc_message_size_filter_init(void) { - grpc_core::MessageSizeParser::Register(); -} - -void grpc_message_size_filter_shutdown(void) {} - namespace grpc_core { void RegisterMessageSizeFilter(CoreConfiguration::Builder* builder) { + MessageSizeParser::Register(builder); builder->channel_init()->RegisterStage( GRPC_CLIENT_SUBCHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY, maybe_add_message_size_filter_subchannel); diff --git a/src/core/ext/filters/message_size/message_size_filter.h b/src/core/ext/filters/message_size/message_size_filter.h index ec011f50498..457ec6efbc0 100644 --- a/src/core/ext/filters/message_size/message_size_filter.h +++ b/src/core/ext/filters/message_size/message_size_filter.h @@ -20,6 +20,7 @@ #include <grpc/support/port_platform.h> #include "src/core/lib/channel/channel_stack.h" +#include "src/core/lib/config/core_configuration.h" #include "src/core/lib/service_config/service_config_parser.h" extern const grpc_channel_filter grpc_message_size_filter; @@ -41,7 +42,8 @@ class MessageSizeParsedConfig : public ServiceConfigParser::ParsedConfig { const message_size_limits& limits() const { return limits_; } static const MessageSizeParsedConfig* GetFromCallContext( - const grpc_call_context_element* context); + const grpc_call_context_element* context, + size_t service_config_parser_index); private: message_size_limits limits_; @@ -49,13 +51,18 @@ class MessageSizeParsedConfig : public ServiceConfigParser::ParsedConfig { class MessageSizeParser : public ServiceConfigParser::Parser { public: + absl::string_view name() const override { return parser_name(); } + std::unique_ptr<ServiceConfigParser::ParsedConfig> ParsePerMethodParams( const grpc_channel_args* /*args*/, const Json& json, grpc_error_handle* error) override; - static void Register(); + static void Register(CoreConfiguration::Builder* builder); static size_t ParserIndex(); + + private: + static absl::string_view parser_name() { return "message_size"; } }; int GetMaxRecvSizeFromChannelArgs(const grpc_channel_args* args); diff --git a/src/core/ext/filters/rbac/rbac_filter.cc b/src/core/ext/filters/rbac/rbac_filter.cc index a3ecc237906..8e9abae06ab 100644 --- a/src/core/ext/filters/rbac/rbac_filter.cc +++ b/src/core/ext/filters/rbac/rbac_filter.cc @@ -70,13 +70,14 @@ void RbacFilter::CallData::RecvInitialMetadataReady(void* user_data, grpc_error_handle error) { grpc_call_element* elem = static_cast<grpc_call_element*>(user_data); CallData* calld = static_cast<CallData*>(elem->call_data); + RbacFilter* filter = static_cast<RbacFilter*>(elem->channel_data); if (error == GRPC_ERROR_NONE) { // Fetch and apply the rbac policy from the service config. auto* service_config_call_data = static_cast<ServiceConfigCallData*>( calld->call_context_[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA].value); auto* method_params = static_cast<RbacMethodParsedConfig*>( service_config_call_data->GetMethodParsedConfig( - RbacServiceConfigParser::ParserIndex())); + filter->service_config_parser_index_)); if (method_params == nullptr) { error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("No RBAC policy found."); } else { @@ -125,6 +126,7 @@ const grpc_channel_filter RbacFilter::kFilterVtable = { RbacFilter::RbacFilter(size_t index, EvaluateArgs::PerChannelArgs per_channel_evaluate_args) : index_(index), + service_config_parser_index_(RbacServiceConfigParser::ParserIndex()), per_channel_evaluate_args_(std::move(per_channel_evaluate_args)) {} grpc_error_handle RbacFilter::Init(grpc_channel_element* elem, @@ -153,8 +155,8 @@ void RbacFilter::Destroy(grpc_channel_element* elem) { chand->~RbacFilter(); } -void RbacFilterInit(void) { RbacServiceConfigParser::Register(); } - -void RbacFilterShutdown(void) {} +void RbacFilterRegister(CoreConfiguration::Builder* builder) { + RbacServiceConfigParser::Register(builder); +} } // namespace grpc_core diff --git a/src/core/ext/filters/rbac/rbac_filter.h b/src/core/ext/filters/rbac/rbac_filter.h index beedcf03609..1c8a31e6854 100644 --- a/src/core/ext/filters/rbac/rbac_filter.h +++ b/src/core/ext/filters/rbac/rbac_filter.h @@ -65,6 +65,8 @@ class RbacFilter { // The index of this filter instance among instances of the same filter. size_t index_; + // Assigned index for service config data from the parser. + const size_t service_config_parser_index_; // Per channel args used for authorization. EvaluateArgs::PerChannelArgs per_channel_evaluate_args_; }; diff --git a/src/core/ext/filters/rbac/rbac_service_config_parser.cc b/src/core/ext/filters/rbac/rbac_service_config_parser.cc index dac1125c19e..08d045abc96 100644 --- a/src/core/ext/filters/rbac/rbac_service_config_parser.cc +++ b/src/core/ext/filters/rbac/rbac_service_config_parser.cc @@ -28,8 +28,6 @@ namespace grpc_core { namespace { -size_t g_rbac_parser_index; - std::string ParseRegexMatcher(const Json::Object& regex_matcher_json, std::vector<grpc_error_handle>* error_list) { std::string regex; @@ -595,11 +593,14 @@ RbacServiceConfigParser::ParsePerMethodParams(const grpc_channel_args* args, return absl::make_unique<RbacMethodParsedConfig>(std::move(rbac_policies)); } -void RbacServiceConfigParser::Register() { - g_rbac_parser_index = ServiceConfigParser::RegisterParser( +void RbacServiceConfigParser::Register(CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( absl::make_unique<RbacServiceConfigParser>()); } -size_t RbacServiceConfigParser::ParserIndex() { return g_rbac_parser_index; } +size_t RbacServiceConfigParser::ParserIndex() { + return CoreConfiguration::Get().service_config_parser().GetParserIndex( + parser_name()); +} } // namespace grpc_core diff --git a/src/core/ext/filters/rbac/rbac_service_config_parser.h b/src/core/ext/filters/rbac/rbac_service_config_parser.h index 96fbec636bb..b9b57a4b342 100644 --- a/src/core/ext/filters/rbac/rbac_service_config_parser.h +++ b/src/core/ext/filters/rbac/rbac_service_config_parser.h @@ -21,6 +21,7 @@ #include <vector> +#include "src/core/lib/config/core_configuration.h" #include "src/core/lib/security/authorization/grpc_authorization_engine.h" #include "src/core/lib/service_config/service_config_parser.h" @@ -55,6 +56,7 @@ class RbacMethodParsedConfig : public ServiceConfigParser::ParsedConfig { class RbacServiceConfigParser : public ServiceConfigParser::Parser { public: + absl::string_view name() const override { return parser_name(); } // Parses the per-method service config for rbac filter. std::unique_ptr<ServiceConfigParser::ParsedConfig> ParsePerMethodParams( const grpc_channel_args* args, const Json& json, @@ -62,7 +64,10 @@ class RbacServiceConfigParser : public ServiceConfigParser::Parser { // Returns the parser index for RbacServiceConfigParser. static size_t ParserIndex(); // Registers RbacServiceConfigParser to ServiceConfigParser. - static void Register(); + static void Register(CoreConfiguration::Builder* builder); + + private: + static absl::string_view parser_name() { return "rbac"; } }; } // namespace grpc_core diff --git a/src/core/ext/transport/cronet/plugin_registry/grpc_cronet_plugin_registry.cc b/src/core/ext/transport/cronet/plugin_registry/grpc_cronet_plugin_registry.cc index 9557b65273b..8df100f17a3 100644 --- a/src/core/ext/transport/cronet/plugin_registry/grpc_cronet_plugin_registry.cc +++ b/src/core/ext/transport/cronet/plugin_registry/grpc_cronet_plugin_registry.cc @@ -29,15 +29,8 @@ void grpc_deadline_filter_shutdown(void); void grpc_client_channel_init(void); void grpc_client_channel_shutdown(void); -namespace grpc_core { -void ServiceConfigParserInit(void); -void ServiceConfigParserShutdown(void); -} // namespace grpc_core - void grpc_register_built_in_plugins(void) { grpc_register_plugin(grpc_http_filters_init, grpc_http_filters_shutdown); - grpc_register_plugin(grpc_core::ServiceConfigParserInit, - grpc_core::ServiceConfigParserShutdown); grpc_register_plugin(grpc_chttp2_plugin_init, grpc_chttp2_plugin_shutdown); grpc_register_plugin(grpc_deadline_filter_init, grpc_deadline_filter_shutdown); diff --git a/src/core/ext/xds/xds_server_config_fetcher.cc b/src/core/ext/xds/xds_server_config_fetcher.cc index e78c35e4a24..a1187a17b96 100644 --- a/src/core/ext/xds/xds_server_config_fetcher.cc +++ b/src/core/ext/xds/xds_server_config_fetcher.cc @@ -37,6 +37,7 @@ #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/iomgr/socket_utils.h" #include "src/core/lib/security/credentials/xds/xds_credentials.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/surface/api_trace.h" #include "src/core/lib/surface/server.h" @@ -1121,7 +1122,7 @@ XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager:: "}"); grpc_error_handle error = GRPC_ERROR_NONE; config_selector_route.method_config = - ServiceConfig::Create(result.args, json.c_str(), &error); + ServiceConfigImpl::Create(result.args, json.c_str(), &error); GPR_ASSERT(error == GRPC_ERROR_NONE); } grpc_channel_args_destroy(result.args); diff --git a/src/core/lib/config/core_configuration.cc b/src/core/lib/config/core_configuration.cc index aeabbc222d1..3924cd95c52 100644 --- a/src/core/lib/config/core_configuration.cc +++ b/src/core/lib/config/core_configuration.cc @@ -16,6 +16,8 @@ #include "src/core/lib/config/core_configuration.h" +#include <atomic> + #include <grpc/support/log.h> namespace grpc_core { @@ -37,6 +39,7 @@ CoreConfiguration::CoreConfiguration(Builder* builder) channel_init_(builder->channel_init_.Build()), handshaker_registry_(builder->handshaker_registry_.Build()), channel_creds_registry_(builder->channel_creds_registry_.Build()), + service_config_parser_(builder->service_config_parser_.Build()), resolver_registry_(builder->resolver_registry_.Build()) {} void CoreConfiguration::RegisterBuilder(std::function<void(Builder*)> builder) { diff --git a/src/core/lib/config/core_configuration.h b/src/core/lib/config/core_configuration.h index e3802050346..a526c79b2e6 100644 --- a/src/core/lib/config/core_configuration.h +++ b/src/core/lib/config/core_configuration.h @@ -23,6 +23,7 @@ #include "src/core/lib/channel/handshaker_registry.h" #include "src/core/lib/resolver/resolver_registry.h" #include "src/core/lib/security/credentials/channel_creds_registry.h" +#include "src/core/lib/service_config/service_config_parser.h" #include "src/core/lib/surface/channel_init.h" namespace grpc_core { @@ -52,6 +53,10 @@ class CoreConfiguration { return &channel_creds_registry_; } + ServiceConfigParser::Builder* service_config_parser() { + return &service_config_parser_; + } + ResolverRegistry::Builder* resolver_registry() { return &resolver_registry_; } @@ -63,6 +68,7 @@ class CoreConfiguration { ChannelInit::Builder channel_init_; HandshakerRegistry::Builder handshaker_registry_; ChannelCredsRegistry<>::Builder channel_creds_registry_; + ServiceConfigParser::Builder service_config_parser_; ResolverRegistry::Builder resolver_registry_; Builder(); @@ -143,6 +149,10 @@ class CoreConfiguration { return channel_creds_registry_; } + const ServiceConfigParser& service_config_parser() const { + return service_config_parser_; + } + const ResolverRegistry& resolver_registry() const { return resolver_registry_; } @@ -175,6 +185,7 @@ class CoreConfiguration { ChannelInit channel_init_; HandshakerRegistry handshaker_registry_; ChannelCredsRegistry<> channel_creds_registry_; + ServiceConfigParser service_config_parser_; ResolverRegistry resolver_registry_; }; diff --git a/src/core/lib/service_config/service_config.h b/src/core/lib/service_config/service_config.h index 9f4a1db5de3..76a882d7011 100644 --- a/src/core/lib/service_config/service_config.h +++ b/src/core/lib/service_config/service_config.h @@ -20,8 +20,7 @@ #include <grpc/support/port_platform.h> #include <unordered_map> - -#include "absl/container/inlined_vector.h" +#include <vector> #include <grpc/impl/codegen/grpc_types.h> #include <grpc/support/string_util.h> @@ -63,63 +62,19 @@ namespace grpc_core { // interface requied to be exposed as part of the resolver API. class ServiceConfig : public RefCounted<ServiceConfig> { public: - /// Creates a new service config from parsing \a json_string. - /// Returns null on parse error. - static RefCountedPtr<ServiceConfig> Create(const grpc_channel_args* args, - absl::string_view json_string, - grpc_error_handle* error); - - ServiceConfig(const grpc_channel_args* args, std::string json_string, - Json json, grpc_error_handle* error); - ~ServiceConfig() override; - - const std::string& json_string() const { return json_string_; } + virtual absl::string_view json_string() const = 0; /// Retrieves the global parsed config at index \a index. The /// lifetime of the returned object is tied to the lifetime of the /// ServiceConfig object. - ServiceConfigParser::ParsedConfig* GetGlobalParsedConfig(size_t index) { - GPR_DEBUG_ASSERT(index < parsed_global_configs_.size()); - return parsed_global_configs_[index].get(); - } + virtual ServiceConfigParser::ParsedConfig* GetGlobalParsedConfig( + size_t index) = 0; /// Retrieves the vector of parsed configs for the method identified /// by \a path. The lifetime of the returned vector and contained objects /// is tied to the lifetime of the ServiceConfig object. - const ServiceConfigParser::ParsedConfigVector* GetMethodParsedConfigVector( - const grpc_slice& path) const; - - private: - // Helper functions for parsing the method configs. - grpc_error_handle ParsePerMethodParams(const grpc_channel_args* args); - grpc_error_handle ParseJsonMethodConfig(const grpc_channel_args* args, - const Json& json); - - // Returns a path string for the JSON name object specified by json. - // Sets *error on error. - static std::string ParseJsonMethodName(const Json& json, - grpc_error_handle* error); - - std::string json_string_; - Json json_; - - absl::InlinedVector<std::unique_ptr<ServiceConfigParser::ParsedConfig>, - ServiceConfigParser::kNumPreallocatedParsers> - parsed_global_configs_; - // A map from the method name to the parsed config vector. Note that we are - // using a raw pointer and not a unique pointer so that we can use the same - // vector for multiple names. - std::unordered_map<grpc_slice, const ServiceConfigParser::ParsedConfigVector*, - SliceHash> - parsed_method_configs_map_; - // Default method config. - const ServiceConfigParser::ParsedConfigVector* default_method_config_vector_ = - nullptr; - // Storage for all the vectors that are being used in - // parsed_method_configs_table_. - absl::InlinedVector<std::unique_ptr<ServiceConfigParser::ParsedConfigVector>, - 32> - parsed_method_config_vectors_storage_; + virtual const ServiceConfigParser::ParsedConfigVector* + GetMethodParsedConfigVector(const grpc_slice& path) const = 0; }; } // namespace grpc_core diff --git a/src/core/lib/service_config/service_config.cc b/src/core/lib/service_config/service_config_impl.cc similarity index 87% rename from src/core/lib/service_config/service_config.cc rename to src/core/lib/service_config/service_config_impl.cc index 1ab2a251481..5f17d6745ad 100644 --- a/src/core/lib/service_config/service_config.cc +++ b/src/core/lib/service_config/service_config_impl.cc @@ -1,5 +1,5 @@ // -// Copyright 2015 gRPC authors. +// Copyright 2022 gRPC authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -16,7 +16,7 @@ #include <grpc/support/port_platform.h> -#include "src/core/lib/service_config/service_config.h" +#include "src/core/lib/service_config/service_config_impl.h" #include <string> @@ -24,25 +24,26 @@ #include <grpc/support/log.h> +#include "src/core/lib/config/core_configuration.h" #include "src/core/lib/json/json.h" #include "src/core/lib/service_config/service_config_parser.h" #include "src/core/lib/slice/slice_internal.h" namespace grpc_core { -RefCountedPtr<ServiceConfig> ServiceConfig::Create( +RefCountedPtr<ServiceConfig> ServiceConfigImpl::Create( const grpc_channel_args* args, absl::string_view json_string, grpc_error_handle* error) { GPR_DEBUG_ASSERT(error != nullptr); Json json = Json::Parse(json_string, error); if (*error != GRPC_ERROR_NONE) return nullptr; - return MakeRefCounted<ServiceConfig>(args, std::string(json_string), - std::move(json), error); + return MakeRefCounted<ServiceConfigImpl>(args, std::string(json_string), + std::move(json), error); } -ServiceConfig::ServiceConfig(const grpc_channel_args* args, - std::string json_string, Json json, - grpc_error_handle* error) +ServiceConfigImpl::ServiceConfigImpl(const grpc_channel_args* args, + std::string json_string, Json json, + grpc_error_handle* error) : json_string_(std::move(json_string)), json_(std::move(json)) { GPR_DEBUG_ASSERT(error != nullptr); if (json_.type() != Json::Type::OBJECT) { @@ -53,7 +54,8 @@ ServiceConfig::ServiceConfig(const grpc_channel_args* args, std::vector<grpc_error_handle> error_list; grpc_error_handle global_error = GRPC_ERROR_NONE; parsed_global_configs_ = - ServiceConfigParser::ParseGlobalParameters(args, json_, &global_error); + CoreConfiguration::Get().service_config_parser().ParseGlobalParameters( + args, json_, &global_error); if (global_error != GRPC_ERROR_NONE) error_list.push_back(global_error); grpc_error_handle local_error = ParsePerMethodParams(args); if (local_error != GRPC_ERROR_NONE) error_list.push_back(local_error); @@ -63,13 +65,13 @@ ServiceConfig::ServiceConfig(const grpc_channel_args* args, } } -ServiceConfig::~ServiceConfig() { +ServiceConfigImpl::~ServiceConfigImpl() { for (auto& p : parsed_method_configs_map_) { grpc_slice_unref_internal(p.first); } } -grpc_error_handle ServiceConfig::ParseJsonMethodConfig( +grpc_error_handle ServiceConfigImpl::ParseJsonMethodConfig( const grpc_channel_args* args, const Json& json) { std::vector<grpc_error_handle> error_list; // Parse method config with each registered parser. @@ -77,7 +79,8 @@ grpc_error_handle ServiceConfig::ParseJsonMethodConfig( absl::make_unique<ServiceConfigParser::ParsedConfigVector>(); grpc_error_handle parser_error = GRPC_ERROR_NONE; *parsed_configs = - ServiceConfigParser::ParsePerMethodParameters(args, json, &parser_error); + CoreConfiguration::Get().service_config_parser().ParsePerMethodParameters( + args, json, &parser_error); if (parser_error != GRPC_ERROR_NONE) { error_list.push_back(parser_error); } @@ -130,7 +133,7 @@ grpc_error_handle ServiceConfig::ParseJsonMethodConfig( return GRPC_ERROR_CREATE_FROM_VECTOR("methodConfig", &error_list); } -grpc_error_handle ServiceConfig::ParsePerMethodParams( +grpc_error_handle ServiceConfigImpl::ParsePerMethodParams( const grpc_channel_args* args) { std::vector<grpc_error_handle> error_list; auto it = json_.object_value().find("methodConfig"); @@ -154,8 +157,8 @@ grpc_error_handle ServiceConfig::ParsePerMethodParams( return GRPC_ERROR_CREATE_FROM_VECTOR("Method Params", &error_list); } -std::string ServiceConfig::ParseJsonMethodName(const Json& json, - grpc_error_handle* error) { +std::string ServiceConfigImpl::ParseJsonMethodName(const Json& json, + grpc_error_handle* error) { if (json.type() != Json::Type::OBJECT) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "field:name error:type is not object"); @@ -204,7 +207,7 @@ std::string ServiceConfig::ParseJsonMethodName(const Json& json, } const ServiceConfigParser::ParsedConfigVector* -ServiceConfig::GetMethodParsedConfigVector(const grpc_slice& path) const { +ServiceConfigImpl::GetMethodParsedConfigVector(const grpc_slice& path) const { if (parsed_method_configs_map_.empty()) { return default_method_config_vector_; } diff --git a/src/core/lib/service_config/service_config_impl.h b/src/core/lib/service_config/service_config_impl.h new file mode 100644 index 00000000000..18b7f6d9195 --- /dev/null +++ b/src/core/lib/service_config/service_config_impl.h @@ -0,0 +1,125 @@ +// +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#ifndef GRPC_CORE_LIB_SERVICE_CONFIG_SERVICE_CONFIG_IMPL_H +#define GRPC_CORE_LIB_SERVICE_CONFIG_SERVICE_CONFIG_IMPL_H + +#include <grpc/support/port_platform.h> + +#include <unordered_map> +#include <vector> + +#include <grpc/impl/codegen/grpc_types.h> +#include <grpc/support/string_util.h> + +#include "src/core/lib/gprpp/ref_counted.h" +#include "src/core/lib/gprpp/ref_counted_ptr.h" +#include "src/core/lib/iomgr/error.h" +#include "src/core/lib/json/json.h" +#include "src/core/lib/service_config/service_config.h" +#include "src/core/lib/service_config/service_config_parser.h" +#include "src/core/lib/slice/slice_internal.h" + +// The main purpose of the code here is to parse the service config in +// JSON form, which will look like this: +// +// { +// "loadBalancingPolicy": "string", // optional +// "methodConfig": [ // array of one or more method_config objects +// { +// "name": [ // array of one or more name objects +// { +// "service": "string", // required +// "method": "string", // optional +// } +// ], +// // remaining fields are optional. +// // see https://developers.google.com/protocol-buffers/docs/proto3#json +// // for format details. +// "waitForReady": bool, +// "timeout": "duration_string", +// "maxRequestMessageBytes": "int64_string", +// "maxResponseMessageBytes": "int64_string", +// } +// ] +// } + +namespace grpc_core { + +class ServiceConfigImpl final : public ServiceConfig { + public: + /// Creates a new service config from parsing \a json_string. + /// Returns null on parse error. + static RefCountedPtr<ServiceConfig> Create(const grpc_channel_args* args, + absl::string_view json_string, + grpc_error_handle* error); + + ServiceConfigImpl(const grpc_channel_args* args, std::string json_string, + Json json, grpc_error_handle* error); + ~ServiceConfigImpl() override; + + absl::string_view json_string() const override { return json_string_; } + + /// Retrieves the global parsed config at index \a index. The + /// lifetime of the returned object is tied to the lifetime of the + /// ServiceConfig object. + ServiceConfigParser::ParsedConfig* GetGlobalParsedConfig( + size_t index) override { + GPR_DEBUG_ASSERT(index < parsed_global_configs_.size()); + return parsed_global_configs_[index].get(); + } + + /// Retrieves the vector of parsed configs for the method identified + /// by \a path. The lifetime of the returned vector and contained objects + /// is tied to the lifetime of the ServiceConfig object. + const ServiceConfigParser::ParsedConfigVector* GetMethodParsedConfigVector( + const grpc_slice& path) const override; + + private: + // Helper functions for parsing the method configs. + grpc_error_handle ParsePerMethodParams(const grpc_channel_args* args); + grpc_error_handle ParseJsonMethodConfig(const grpc_channel_args* args, + const Json& json); + + // Returns a path string for the JSON name object specified by json. + // Sets *error on error. + static std::string ParseJsonMethodName(const Json& json, + grpc_error_handle* error); + + std::string json_string_; + Json json_; + + std::vector<std::unique_ptr<ServiceConfigParser::ParsedConfig>> + parsed_global_configs_; + // A map from the method name to the parsed config vector. Note that we are + // using a raw pointer and not a unique pointer so that we can use the same + // vector for multiple names. + std::unordered_map<grpc_slice, const ServiceConfigParser::ParsedConfigVector*, + SliceHash> + parsed_method_configs_map_; + // Default method config. + const ServiceConfigParser::ParsedConfigVector* default_method_config_vector_ = + nullptr; + // Storage for all the vectors that are being used in + // parsed_method_configs_table_. + absl::InlinedVector<std::unique_ptr<ServiceConfigParser::ParsedConfigVector>, + 32> + parsed_method_config_vectors_storage_; +}; + +} // namespace grpc_core + +#endif /* GRPC_CORE_LIB_SERVICE_CONFIG_SERVICE_CONFIG_IMPL_H */ diff --git a/src/core/lib/service_config/service_config_parser.cc b/src/core/lib/service_config/service_config_parser.cc index edd8dfc86d9..569eef67b7b 100644 --- a/src/core/lib/service_config/service_config_parser.cc +++ b/src/core/lib/service_config/service_config_parser.cc @@ -22,38 +22,35 @@ namespace grpc_core { -namespace { -typedef absl::InlinedVector<std::unique_ptr<ServiceConfigParser::Parser>, - ServiceConfigParser::kNumPreallocatedParsers> - ServiceConfigParserList; -ServiceConfigParserList* g_registered_parsers; -} // namespace - -void ServiceConfigParserInit() { - GPR_ASSERT(g_registered_parsers == nullptr); - g_registered_parsers = new ServiceConfigParserList(); -} - -void ServiceConfigParserShutdown() { - delete g_registered_parsers; - g_registered_parsers = nullptr; +ServiceConfigParser ServiceConfigParser::Builder::Build() { + return ServiceConfigParser(std::move(registered_parsers_)); } -size_t ServiceConfigParser::RegisterParser(std::unique_ptr<Parser> parser) { - g_registered_parsers->push_back(std::move(parser)); - return g_registered_parsers->size() - 1; +void ServiceConfigParser::Builder::RegisterParser( + std::unique_ptr<Parser> parser) { + for (const auto& registered_parser : registered_parsers_) { + if (registered_parser->name() == parser->name()) { + gpr_log(GPR_ERROR, "%s", + absl::StrCat("Parser with name '", parser->name(), + "' already registered") + .c_str()); + // We'll otherwise crash later. + abort(); + } + } + registered_parsers_.emplace_back(std::move(parser)); } ServiceConfigParser::ParsedConfigVector ServiceConfigParser::ParseGlobalParameters(const grpc_channel_args* args, const Json& json, - grpc_error_handle* error) { + grpc_error_handle* error) const { ParsedConfigVector parsed_global_configs; std::vector<grpc_error_handle> error_list; - for (size_t i = 0; i < g_registered_parsers->size(); i++) { + for (size_t i = 0; i < registered_parsers_.size(); i++) { grpc_error_handle parser_error = GRPC_ERROR_NONE; - auto parsed_config = (*g_registered_parsers)[i]->ParseGlobalParams( - args, json, &parser_error); + auto parsed_config = + registered_parsers_[i]->ParseGlobalParams(args, json, &parser_error); if (parser_error != GRPC_ERROR_NONE) { error_list.push_back(parser_error); } @@ -68,13 +65,13 @@ ServiceConfigParser::ParseGlobalParameters(const grpc_channel_args* args, ServiceConfigParser::ParsedConfigVector ServiceConfigParser::ParsePerMethodParameters(const grpc_channel_args* args, const Json& json, - grpc_error_handle* error) { + grpc_error_handle* error) const { ParsedConfigVector parsed_method_configs; std::vector<grpc_error_handle> error_list; - for (size_t i = 0; i < g_registered_parsers->size(); i++) { + for (size_t i = 0; i < registered_parsers_.size(); ++i) { grpc_error_handle parser_error = GRPC_ERROR_NONE; - auto parsed_config = (*g_registered_parsers)[i]->ParsePerMethodParams( - args, json, &parser_error); + auto parsed_config = + registered_parsers_[i]->ParsePerMethodParams(args, json, &parser_error); if (parser_error != GRPC_ERROR_NONE) { error_list.push_back(parser_error); } @@ -86,4 +83,11 @@ ServiceConfigParser::ParsePerMethodParameters(const grpc_channel_args* args, return parsed_method_configs; } +size_t ServiceConfigParser::GetParserIndex(absl::string_view name) const { + for (size_t i = 0; i < registered_parsers_.size(); ++i) { + if (registered_parsers_[i]->name() == name) return i; + } + return -1; +} + } // namespace grpc_core diff --git a/src/core/lib/service_config/service_config_parser.h b/src/core/lib/service_config/service_config_parser.h index b1e664db05b..eabcdff3020 100644 --- a/src/core/lib/service_config/service_config_parser.h +++ b/src/core/lib/service_config/service_config_parser.h @@ -20,8 +20,7 @@ #include <grpc/support/port_platform.h> #include <memory> - -#include "absl/container/inlined_vector.h" +#include <vector> #include <grpc/impl/codegen/grpc_types.h> @@ -30,11 +29,6 @@ namespace grpc_core { -// Initialization functions for ServiceConfigParser. ServiceConfigParser should -// be initialized before any parser implementation is registered. -void ServiceConfigParserInit(); -void ServiceConfigParserShutdown(); - // Service config parser registry. // See service_config.h for more information. class ServiceConfigParser { @@ -51,6 +45,8 @@ class ServiceConfigParser { public: virtual ~Parser() = default; + virtual absl::string_view name() const = 0; + virtual std::unique_ptr<ParsedConfig> ParseGlobalParams( const grpc_channel_args*, const Json& /* json */, grpc_error_handle* error) { @@ -70,26 +66,39 @@ class ServiceConfigParser { } }; - static constexpr int kNumPreallocatedParsers = 4; - typedef absl::InlinedVector<std::unique_ptr<ParsedConfig>, - kNumPreallocatedParsers> - ParsedConfigVector; - - /// Globally register a service config parser. On successful registration, it - /// returns the index at which the parser was registered. On failure, -1 is - /// returned. Each new service config update will go through all the - /// registered parser. Each parser is responsible for reading the service - /// config json and returning a parsed config. This parsed config can later be - /// retrieved using the same index that was returned at registration time. - static size_t RegisterParser(std::unique_ptr<Parser> parser); - - static ParsedConfigVector ParseGlobalParameters(const grpc_channel_args* args, - const Json& json, - grpc_error_handle* error); - - static ParsedConfigVector ParsePerMethodParameters( - const grpc_channel_args* args, const Json& json, - grpc_error_handle* error); + using ServiceConfigParserList = std::vector<std::unique_ptr<Parser>>; + using ParsedConfigVector = std::vector<std::unique_ptr<ParsedConfig>>; + + class Builder { + public: + /// Globally register a service config parser. Each new service config + /// update will go through all the registered parser. Each parser is + /// responsible for reading the service config json and returning a parsed + /// config. + void RegisterParser(std::unique_ptr<Parser> parser); + + ServiceConfigParser Build(); + + private: + ServiceConfigParserList registered_parsers_; + }; + + ParsedConfigVector ParseGlobalParameters(const grpc_channel_args* args, + const Json& json, + grpc_error_handle* error) const; + + ParsedConfigVector ParsePerMethodParameters(const grpc_channel_args* args, + const Json& json, + grpc_error_handle* error) const; + + // Return the index for a given registered parser. + // If there is an error, return -1. + size_t GetParserIndex(absl::string_view name) const; + + private: + explicit ServiceConfigParser(ServiceConfigParserList registered_parsers) + : registered_parsers_(std::move(registered_parsers)) {} + ServiceConfigParserList registered_parsers_; }; } // namespace grpc_core diff --git a/src/core/plugin_registry/grpc_plugin_registry.cc b/src/core/plugin_registry/grpc_plugin_registry.cc index a3c35c0aeb2..aafcd98362e 100644 --- a/src/core/plugin_registry/grpc_plugin_registry.cc +++ b/src/core/plugin_registry/grpc_plugin_registry.cc @@ -41,24 +41,16 @@ void grpc_lb_policy_round_robin_init(void); void grpc_lb_policy_round_robin_shutdown(void); void grpc_resolver_dns_ares_init(void); void grpc_resolver_dns_ares_shutdown(void); -void grpc_message_size_filter_init(void); -void grpc_message_size_filter_shutdown(void); namespace grpc_core { -void FaultInjectionFilterInit(void); -void FaultInjectionFilterShutdown(void); void GrpcLbPolicyRingHashInit(void); void GrpcLbPolicyRingHashShutdown(void); #ifndef GRPC_NO_RLS void RlsLbPluginInit(); void RlsLbPluginShutdown(); #endif // !GRPC_NO_RLS -void ServiceConfigParserInit(void); -void ServiceConfigParserShutdown(void); } // namespace grpc_core void grpc_register_built_in_plugins(void) { - grpc_register_plugin(grpc_core::ServiceConfigParserInit, - grpc_core::ServiceConfigParserShutdown); grpc_register_plugin(grpc_client_channel_init, grpc_client_channel_shutdown); grpc_register_plugin(grpc_lb_policy_grpclb_init, grpc_lb_policy_grpclb_shutdown); @@ -78,10 +70,6 @@ void grpc_register_built_in_plugins(void) { grpc_core::GrpcLbPolicyRingHashShutdown); grpc_register_plugin(grpc_resolver_dns_ares_init, grpc_resolver_dns_ares_shutdown); - grpc_register_plugin(grpc_message_size_filter_init, - grpc_message_size_filter_shutdown); - grpc_register_plugin(grpc_core::FaultInjectionFilterInit, - grpc_core::FaultInjectionFilterShutdown); grpc_register_extra_plugins(); } @@ -104,6 +92,7 @@ extern void RegisterServiceConfigChannelArgFilter( CoreConfiguration::Builder* builder); extern void RegisterExtraFilters(CoreConfiguration::Builder* builder); extern void RegisterResourceQuota(CoreConfiguration::Builder* builder); +extern void FaultInjectionFilterRegister(CoreConfiguration::Builder* builder); extern void RegisterNativeDnsResolver(CoreConfiguration::Builder* builder); extern void RegisterAresDnsResolver(CoreConfiguration::Builder* builder); extern void RegisterSockaddrResolver(CoreConfiguration::Builder* builder); @@ -124,6 +113,7 @@ void BuildCoreConfiguration(CoreConfiguration::Builder* builder) { RegisterMessageSizeFilter(builder); RegisterServiceConfigChannelArgFilter(builder); RegisterResourceQuota(builder); + FaultInjectionFilterRegister(builder); RegisterAresDnsResolver(builder); RegisterNativeDnsResolver(builder); RegisterSockaddrResolver(builder); diff --git a/src/core/plugin_registry/grpc_plugin_registry_extra.cc b/src/core/plugin_registry/grpc_plugin_registry_extra.cc index 87c092976bf..68e792ac8c4 100644 --- a/src/core/plugin_registry/grpc_plugin_registry_extra.cc +++ b/src/core/plugin_registry/grpc_plugin_registry_extra.cc @@ -23,8 +23,6 @@ #ifndef GRPC_NO_XDS namespace grpc_core { -void RbacFilterInit(void); -void RbacFilterShutdown(void); void XdsClientGlobalInit(); void XdsClientGlobalShutdown(); } // namespace grpc_core @@ -46,10 +44,6 @@ void grpc_lb_policy_xds_cluster_manager_shutdown(void); void grpc_register_extra_plugins() { #ifndef GRPC_NO_XDS - // rbac_filter is being guarded with GRPC_NO_XDS to avoid a dependency on the - // re2 library by default - grpc_register_plugin(grpc_core::RbacFilterInit, - grpc_core::RbacFilterShutdown); grpc_register_plugin(grpc_core::XdsClientGlobalInit, grpc_core::XdsClientGlobalShutdown); grpc_register_plugin(grpc_certificate_provider_registry_init, @@ -68,6 +62,7 @@ void grpc_register_extra_plugins() { namespace grpc_core { #ifndef GRPC_NO_XDS +extern void RbacFilterRegister(CoreConfiguration::Builder* builder); extern void RegisterXdsChannelStackModifier( CoreConfiguration::Builder* builder); extern void RegisterChannelDefaultCreds(CoreConfiguration::Builder* builder); @@ -78,6 +73,9 @@ void RegisterExtraFilters(CoreConfiguration::Builder* builder) { // Use builder to avoid unused-parameter warning. (void)builder; #ifndef GRPC_NO_XDS + // rbac_filter is being guarded with GRPC_NO_XDS to avoid a dependency on the + // re2 library by default + RbacFilterRegister(builder); RegisterXdsChannelStackModifier(builder); RegisterChannelDefaultCreds(builder); RegisterXdsResolver(builder); diff --git a/src/cpp/common/validate_service_config.cc b/src/cpp/common/validate_service_config.cc index a86a2c2b305..c444ea2d62e 100644 --- a/src/cpp/common/validate_service_config.cc +++ b/src/cpp/common/validate_service_config.cc @@ -19,15 +19,15 @@ #include <grpc/grpc.h> #include <grpcpp/support/validate_service_config.h> -#include "src/core/lib/service_config/service_config.h" +#include "src/core/lib/service_config/service_config_impl.h" namespace grpc { namespace experimental { std::string ValidateServiceConfigJSON(const std::string& service_config_json) { grpc_init(); grpc_error_handle error = GRPC_ERROR_NONE; - grpc_core::ServiceConfig::Create(/*args=*/nullptr, - service_config_json.c_str(), &error); + grpc_core::ServiceConfigImpl::Create(/*args=*/nullptr, + service_config_json.c_str(), &error); std::string return_value; if (error != GRPC_ERROR_NONE) { return_value = grpc_error_std_string(error); diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 7d946bed542..b287838fab7 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -647,7 +647,7 @@ CORE_SOURCE_FILES = [ 'src/core/lib/security/transport/server_auth_filter.cc', 'src/core/lib/security/transport/tsi_error.cc', 'src/core/lib/security/util/json_util.cc', - 'src/core/lib/service_config/service_config.cc', + 'src/core/lib/service_config/service_config_impl.cc', 'src/core/lib/service_config/service_config_parser.cc', 'src/core/lib/slice/b64.cc', 'src/core/lib/slice/percent_encoding.cc', diff --git a/test/core/client_channel/rls_lb_config_parser_test.cc b/test/core/client_channel/rls_lb_config_parser_test.cc index 9086271b7f4..3089e33c988 100644 --- a/test/core/client_channel/rls_lb_config_parser_test.cc +++ b/test/core/client_channel/rls_lb_config_parser_test.cc @@ -20,7 +20,7 @@ #include <grpc/grpc.h> #include "src/core/lib/gpr/env.h" -#include "src/core/lib/service_config/service_config.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "test/core/util/test_config.h" // A regular expression to enter referenced or child errors. @@ -74,7 +74,7 @@ TEST_F(RlsConfigParsingTest, ValidConfig) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); EXPECT_NE(service_config, nullptr); @@ -93,7 +93,7 @@ TEST_F(RlsConfigParsingTest, TopLevelRequiredFieldsMissing) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT( grpc_error_std_string(error), @@ -118,7 +118,7 @@ TEST_F(RlsConfigParsingTest, TopLevelFieldsWrongTypes) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT( grpc_error_std_string(error), @@ -144,7 +144,7 @@ TEST_F(RlsConfigParsingTest, TopLevelFieldsInvalidValues) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT( grpc_error_std_string(error), @@ -169,7 +169,7 @@ TEST_F(RlsConfigParsingTest, InvalidChildPolicyConfig) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT( grpc_error_std_string(error), @@ -196,7 +196,7 @@ TEST_F(RlsConfigParsingTest, InvalidRlsChannelServiceConfig) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( @@ -224,7 +224,7 @@ TEST_F(RlsConfigParsingTest, RouteLookupConfigRequiredFieldsMissing) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( @@ -254,7 +254,7 @@ TEST_F(RlsConfigParsingTest, RouteLookupConfigFieldsWrongTypes) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( @@ -282,7 +282,7 @@ TEST_F(RlsConfigParsingTest, RouteLookupConfigFieldsInvalidValues) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( @@ -312,7 +312,7 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderRequiredFieldsMissing) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT( grpc_error_std_string(error), @@ -343,7 +343,7 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderWrongFieldTypes) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT( grpc_error_std_string(error), @@ -382,7 +382,7 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderInvalidValues) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( @@ -434,7 +434,7 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderInvalidHeaders) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT( grpc_error_std_string(error), @@ -481,7 +481,7 @@ TEST_F(RlsConfigParsingTest, GrpcKeybuilderNameWrongFieldTypes) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT( grpc_error_std_string(error), @@ -521,7 +521,7 @@ TEST_F(RlsConfigParsingTest, DuplicateMethodNamesInSameKeyBuilder) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT( grpc_error_std_string(error), @@ -562,7 +562,7 @@ TEST_F(RlsConfigParsingTest, DuplicateMethodNamesInDifferentKeyBuilders) { " }]\n" "}\n"; grpc_error_handle error = GRPC_ERROR_NONE; - auto service_config = ServiceConfig::Create( + auto service_config = ServiceConfigImpl::Create( /*args=*/nullptr, service_config_json, &error); EXPECT_THAT( grpc_error_std_string(error), diff --git a/test/core/client_channel/service_config_test.cc b/test/core/client_channel/service_config_test.cc index d0128193248..746f43f250b 100644 --- a/test/core/client_channel/service_config_test.cc +++ b/test/core/client_channel/service_config_test.cc @@ -29,6 +29,7 @@ #include "src/core/ext/filters/client_channel/retry_service_config.h" #include "src/core/ext/filters/message_size/message_size_filter.h" #include "src/core/lib/gpr/string.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/core/lib/service_config/service_config_parser.h" #include "test/core/util/port.h" #include "test/core/util/test_config.h" @@ -62,6 +63,8 @@ class TestParsedConfig1 : public ServiceConfigParser::ParsedConfig { class TestParser1 : public ServiceConfigParser::Parser { public: + absl::string_view name() const override { return "test_parser_1"; } + std::unique_ptr<ServiceConfigParser::ParsedConfig> ParseGlobalParams( const grpc_channel_args* args, const Json& json, grpc_error_handle* error) override { @@ -98,6 +101,8 @@ class TestParser1 : public ServiceConfigParser::Parser { class TestParser2 : public ServiceConfigParser::Parser { public: + absl::string_view name() const override { return "test_parser_2"; } + std::unique_ptr<ServiceConfigParser::ParsedConfig> ParsePerMethodParams( const grpc_channel_args* args, const Json& json, grpc_error_handle* error) override { @@ -135,6 +140,10 @@ class TestParser2 : public ServiceConfigParser::Parser { // This parser always adds errors class ErrorParser : public ServiceConfigParser::Parser { public: + explicit ErrorParser(absl::string_view name) : name_(name) {} + + absl::string_view name() const override { return name_; } + std::unique_ptr<ServiceConfigParser::ParsedConfig> ParsePerMethodParams( const grpc_channel_args* /*arg*/, const Json& /*json*/, grpc_error_handle* error) override { @@ -154,26 +163,35 @@ class ErrorParser : public ServiceConfigParser::Parser { static const char* MethodError() { return "ErrorParser : methodError"; } static const char* GlobalError() { return "ErrorParser : globalError"; } + + private: + absl::string_view name_; }; class ServiceConfigTest : public ::testing::Test { protected: void SetUp() override { - ServiceConfigParserShutdown(); - ServiceConfigParserInit(); - EXPECT_EQ( - ServiceConfigParser::RegisterParser(absl::make_unique<TestParser1>()), - 0); - EXPECT_EQ( - ServiceConfigParser::RegisterParser(absl::make_unique<TestParser2>()), - 1); + CoreConfiguration::Reset(); + CoreConfiguration::BuildSpecialConfiguration( + [](CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( + absl::make_unique<TestParser1>()); + builder->service_config_parser()->RegisterParser( + absl::make_unique<TestParser2>()); + }); + EXPECT_EQ(CoreConfiguration::Get().service_config_parser().GetParserIndex( + "test_parser_1"), + 0); + EXPECT_EQ(CoreConfiguration::Get().service_config_parser().GetParserIndex( + "test_parser_2"), + 1); } }; TEST_F(ServiceConfigTest, ErrorCheck1) { const char* test_json = ""; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex("JSON parse error")); GRPC_ERROR_UNREF(error); @@ -182,7 +200,7 @@ TEST_F(ServiceConfigTest, ErrorCheck1) { TEST_F(ServiceConfigTest, BasicTest1) { const char* test_json = "{}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); } @@ -194,7 +212,7 @@ TEST_F(ServiceConfigTest, SkipMethodConfigWithNoNameOrEmptyName) { " {\"name\":[{\"service\":\"TestServ\"}], \"method_param\":2}" "]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( grpc_slice_from_static_string("/TestServ/TestMethod")); @@ -210,7 +228,7 @@ TEST_F(ServiceConfigTest, ErrorDuplicateMethodConfigNames) { " {\"name\":[{\"service\":\"TestServ\"}]}" "]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -226,7 +244,7 @@ TEST_F(ServiceConfigTest, ErrorDuplicateMethodConfigNamesWithNullMethod) { " {\"name\":[{\"service\":\"TestServ\"}]}" "]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -242,7 +260,7 @@ TEST_F(ServiceConfigTest, ErrorDuplicateMethodConfigNamesWithEmptyMethod) { " {\"name\":[{\"service\":\"TestServ\"}]}" "]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -258,7 +276,7 @@ TEST_F(ServiceConfigTest, ErrorDuplicateDefaultMethodConfigs) { " {\"name\":[{}]}" "]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -274,7 +292,7 @@ TEST_F(ServiceConfigTest, ErrorDuplicateDefaultMethodConfigsWithNullService) { " {\"name\":[{}]}" "]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -290,7 +308,7 @@ TEST_F(ServiceConfigTest, ErrorDuplicateDefaultMethodConfigsWithEmptyService) { " {\"name\":[{}]}" "]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -303,14 +321,14 @@ TEST_F(ServiceConfigTest, ValidMethodConfig) { const char* test_json = "{\"methodConfig\": [{\"name\":[{\"service\":\"TestServ\"}]}]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); } TEST_F(ServiceConfigTest, Parser1BasicTest1) { const char* test_json = "{\"global_param\":5}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); EXPECT_EQ((static_cast<TestParsedConfig1*>(svc_cfg->GetGlobalParsedConfig(0))) ->value(), @@ -323,7 +341,7 @@ TEST_F(ServiceConfigTest, Parser1BasicTest1) { TEST_F(ServiceConfigTest, Parser1BasicTest2) { const char* test_json = "{\"global_param\":1000}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); EXPECT_EQ((static_cast<TestParsedConfig1*>(svc_cfg->GetGlobalParsedConfig(0))) ->value(), @@ -336,7 +354,7 @@ TEST_F(ServiceConfigTest, Parser1DisabledViaChannelArg) { grpc_channel_args args = {1, &arg}; const char* test_json = "{\"global_param\":5}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); EXPECT_EQ(svc_cfg->GetGlobalParsedConfig(0), nullptr); } @@ -344,7 +362,7 @@ TEST_F(ServiceConfigTest, Parser1DisabledViaChannelArg) { TEST_F(ServiceConfigTest, Parser1ErrorInvalidType) { const char* test_json = "{\"global_param\":\"5\"}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( absl::StrCat("Service config parsing error" CHILD_ERROR_TAG @@ -356,7 +374,7 @@ TEST_F(ServiceConfigTest, Parser1ErrorInvalidType) { TEST_F(ServiceConfigTest, Parser1ErrorInvalidValue) { const char* test_json = "{\"global_param\":-5}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( absl::StrCat("Service config parsing error" CHILD_ERROR_TAG @@ -370,7 +388,7 @@ TEST_F(ServiceConfigTest, Parser2BasicTest) { "{\"methodConfig\": [{\"name\":[{\"service\":\"TestServ\"}], " "\"method_param\":5}]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( grpc_slice_from_static_string("/TestServ/TestMethod")); @@ -387,7 +405,7 @@ TEST_F(ServiceConfigTest, Parser2DisabledViaChannelArg) { "{\"methodConfig\": [{\"name\":[{\"service\":\"TestServ\"}], " "\"method_param\":5}]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( grpc_slice_from_static_string("/TestServ/TestMethod")); @@ -401,7 +419,7 @@ TEST_F(ServiceConfigTest, Parser2ErrorInvalidType) { "{\"methodConfig\": [{\"name\":[{\"service\":\"TestServ\"}], " "\"method_param\":\"5\"}]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex(absl::StrCat( @@ -416,7 +434,7 @@ TEST_F(ServiceConfigTest, Parser2ErrorInvalidValue) { "{\"methodConfig\": [{\"name\":[{\"service\":\"TestServ\"}], " "\"method_param\":-5}]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex(absl::StrCat( @@ -426,17 +444,36 @@ TEST_F(ServiceConfigTest, Parser2ErrorInvalidValue) { GRPC_ERROR_UNREF(error); } +TEST(ServiceConfigParserTest, DoubleRegistration) { + CoreConfiguration::Reset(); + ASSERT_DEATH_IF_SUPPORTED( + CoreConfiguration::BuildSpecialConfiguration( + [](CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( + absl::make_unique<ErrorParser>("xyzabc")); + builder->service_config_parser()->RegisterParser( + absl::make_unique<ErrorParser>("xyzabc")); + }), + "xyzabc.*already registered"); +} + // Test parsing with ErrorParsers which always add errors class ErroredParsersScopingTest : public ::testing::Test { protected: void SetUp() override { - ServiceConfigParserShutdown(); - ServiceConfigParserInit(); + CoreConfiguration::Reset(); + CoreConfiguration::BuildSpecialConfiguration( + [](CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( + absl::make_unique<ErrorParser>("ep1")); + builder->service_config_parser()->RegisterParser( + absl::make_unique<ErrorParser>("ep2")); + }); EXPECT_EQ( - ServiceConfigParser::RegisterParser(absl::make_unique<ErrorParser>()), + CoreConfiguration::Get().service_config_parser().GetParserIndex("ep1"), 0); EXPECT_EQ( - ServiceConfigParser::RegisterParser(absl::make_unique<ErrorParser>()), + CoreConfiguration::Get().service_config_parser().GetParserIndex("ep2"), 1); } }; @@ -444,7 +481,7 @@ class ErroredParsersScopingTest : public ::testing::Test { TEST_F(ErroredParsersScopingTest, GlobalParams) { const char* test_json = "{}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex(absl::StrCat( @@ -457,7 +494,7 @@ TEST_F(ErroredParsersScopingTest, GlobalParams) { TEST_F(ErroredParsersScopingTest, MethodParams) { const char* test_json = "{\"methodConfig\": [{}]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex(absl::StrCat( @@ -476,19 +513,22 @@ TEST_F(ErroredParsersScopingTest, MethodParams) { class ClientChannelParserTest : public ::testing::Test { protected: void SetUp() override { - ServiceConfigParserShutdown(); - ServiceConfigParserInit(); - EXPECT_EQ( - ServiceConfigParser::RegisterParser( - absl::make_unique<internal::ClientChannelServiceConfigParser>()), - 0); + CoreConfiguration::Reset(); + CoreConfiguration::BuildSpecialConfiguration( + [](CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( + absl::make_unique<internal::ClientChannelServiceConfigParser>()); + }); + EXPECT_EQ(CoreConfiguration::Get().service_config_parser().GetParserIndex( + "client_channel"), + 0); } }; TEST_F(ClientChannelParserTest, ValidLoadBalancingConfigPickFirst) { const char* test_json = "{\"loadBalancingConfig\": [{\"pick_first\":{}}]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* parsed_config = static_cast<internal::ClientChannelGlobalParsedConfig*>( @@ -501,7 +541,7 @@ TEST_F(ClientChannelParserTest, ValidLoadBalancingConfigRoundRobin) { const char* test_json = "{\"loadBalancingConfig\": [{\"round_robin\":{}}, {}]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); auto parsed_config = static_cast<internal::ClientChannelGlobalParsedConfig*>( svc_cfg->GetGlobalParsedConfig(0)); @@ -514,7 +554,7 @@ TEST_F(ClientChannelParserTest, ValidLoadBalancingConfigGrpclb) { "{\"loadBalancingConfig\": " "[{\"grpclb\":{\"childPolicy\":[{\"pick_first\":{}}]}}]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* parsed_config = static_cast<internal::ClientChannelGlobalParsedConfig*>( @@ -537,7 +577,7 @@ TEST_F(ClientChannelParserTest, ValidLoadBalancingConfigXds) { " ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* parsed_config = static_cast<internal::ClientChannelGlobalParsedConfig*>( @@ -549,7 +589,7 @@ TEST_F(ClientChannelParserTest, ValidLoadBalancingConfigXds) { TEST_F(ClientChannelParserTest, UnknownLoadBalancingConfig) { const char* test_json = "{\"loadBalancingConfig\": [{\"unknown\":{}}]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex("Service config parsing error" CHILD_ERROR_TAG @@ -567,7 +607,7 @@ TEST_F(ClientChannelParserTest, InvalidGrpclbLoadBalancingConfig) { " {\"round_robin\":{}}" "]}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -582,7 +622,7 @@ TEST_F(ClientChannelParserTest, InvalidGrpclbLoadBalancingConfig) { TEST_F(ClientChannelParserTest, ValidLoadBalancingPolicy) { const char* test_json = "{\"loadBalancingPolicy\":\"pick_first\"}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* parsed_config = static_cast<internal::ClientChannelGlobalParsedConfig*>( @@ -593,7 +633,7 @@ TEST_F(ClientChannelParserTest, ValidLoadBalancingPolicy) { TEST_F(ClientChannelParserTest, ValidLoadBalancingPolicyAllCaps) { const char* test_json = "{\"loadBalancingPolicy\":\"PICK_FIRST\"}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* parsed_config = static_cast<internal::ClientChannelGlobalParsedConfig*>( @@ -604,7 +644,7 @@ TEST_F(ClientChannelParserTest, ValidLoadBalancingPolicyAllCaps) { TEST_F(ClientChannelParserTest, UnknownLoadBalancingPolicy) { const char* test_json = "{\"loadBalancingPolicy\":\"unknown\"}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -618,7 +658,7 @@ TEST_F(ClientChannelParserTest, LoadBalancingPolicyXdsNotAllowed) { const char* test_json = "{\"loadBalancingPolicy\":\"xds_cluster_resolver_experimental\"}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -641,7 +681,7 @@ TEST_F(ClientChannelParserTest, ValidTimeout) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( grpc_slice_from_static_string("/TestServ/TestMethod")); @@ -664,7 +704,7 @@ TEST_F(ClientChannelParserTest, InvalidTimeout) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -686,7 +726,7 @@ TEST_F(ClientChannelParserTest, ValidWaitForReady) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( grpc_slice_from_static_string("/TestServ/TestMethod")); @@ -713,7 +753,7 @@ TEST_F(ClientChannelParserTest, InvalidWaitForReady) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -731,7 +771,7 @@ TEST_F(ClientChannelParserTest, ValidHealthCheck) { " }\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* parsed_config = static_cast<internal::ClientChannelGlobalParsedConfig*>( @@ -752,7 +792,7 @@ TEST_F(ClientChannelParserTest, InvalidHealthCheckMultipleEntries) { " }\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "JSON parsing failed" CHILD_ERROR_TAG @@ -767,10 +807,14 @@ TEST_F(ClientChannelParserTest, InvalidHealthCheckMultipleEntries) { class RetryParserTest : public ::testing::Test { protected: void SetUp() override { - ServiceConfigParserShutdown(); - ServiceConfigParserInit(); - EXPECT_EQ(ServiceConfigParser::RegisterParser( - absl::make_unique<internal::RetryServiceConfigParser>()), + CoreConfiguration::Reset(); + CoreConfiguration::BuildSpecialConfiguration( + [](CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( + absl::make_unique<internal::RetryServiceConfigParser>()); + }); + EXPECT_EQ(CoreConfiguration::Get().service_config_parser().GetParserIndex( + "retry"), 0); } }; @@ -784,7 +828,7 @@ TEST_F(RetryParserTest, ValidRetryThrottling) { " }\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* parsed_config = static_cast<internal::RetryGlobalConfig*>( svc_cfg->GetGlobalParsedConfig(0)); @@ -800,7 +844,7 @@ TEST_F(RetryParserTest, RetryThrottlingMissingFields) { " }\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex( @@ -820,7 +864,7 @@ TEST_F(RetryParserTest, InvalidRetryThrottlingNegativeMaxTokens) { " }\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex( @@ -840,7 +884,7 @@ TEST_F(RetryParserTest, InvalidRetryThrottlingInvalidTokenRatio) { " }\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex("Service config parsing error" CHILD_ERROR_TAG @@ -868,7 +912,7 @@ TEST_F(RetryParserTest, ValidRetryPolicy) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( grpc_slice_from_static_string("/TestServ/TestMethod")); @@ -896,7 +940,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyWrongType) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -918,7 +962,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyRequiredFieldsMissing) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -948,7 +992,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyMaxAttemptsWrongType) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -975,7 +1019,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyMaxAttemptsBadValue) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1002,7 +1046,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyInitialBackoffWrongType) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1030,7 +1074,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyInitialBackoffBadValue) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1057,7 +1101,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyMaxBackoffWrongType) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1085,7 +1129,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyMaxBackoffBadValue) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1112,7 +1156,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyBackoffMultiplierWrongType) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1139,7 +1183,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyBackoffMultiplierBadValue) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1166,7 +1210,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyEmptyRetryableStatusCodes) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1193,7 +1237,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyRetryableStatusCodesWrongType) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1220,7 +1264,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyUnparseableRetryableStatusCodes) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1253,7 +1297,7 @@ TEST_F(RetryParserTest, ValidRetryPolicyWithPerAttemptRecvTimeout) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( grpc_slice_from_static_string("/TestServ/TestMethod")); @@ -1289,7 +1333,7 @@ TEST_F(RetryParserTest, " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( grpc_slice_from_static_string("/TestServ/TestMethod")); @@ -1327,7 +1371,7 @@ TEST_F(RetryParserTest, grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( grpc_slice_from_static_string("/TestServ/TestMethod")); @@ -1364,7 +1408,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyPerAttemptRecvTimeoutUnparseable) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1396,7 +1440,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyPerAttemptRecvTimeoutWrongType) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1428,7 +1472,7 @@ TEST_F(RetryParserTest, InvalidRetryPolicyPerAttemptRecvTimeoutBadValue) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_EXPERIMENTAL_ENABLE_HEDGING), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1445,10 +1489,14 @@ TEST_F(RetryParserTest, InvalidRetryPolicyPerAttemptRecvTimeoutBadValue) { class MessageSizeParserTest : public ::testing::Test { protected: void SetUp() override { - ServiceConfigParserShutdown(); - ServiceConfigParserInit(); - EXPECT_EQ(ServiceConfigParser::RegisterParser( - absl::make_unique<MessageSizeParser>()), + CoreConfiguration::Reset(); + CoreConfiguration::BuildSpecialConfiguration( + [](CoreConfiguration::Builder* builder) { + builder->service_config_parser()->RegisterParser( + absl::make_unique<MessageSizeParser>()); + }); + EXPECT_EQ(CoreConfiguration::Get().service_config_parser().GetParserIndex( + "message_size"), 0); } }; @@ -1465,7 +1513,7 @@ TEST_F(MessageSizeParserTest, Valid) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector( grpc_slice_from_static_string("/TestServ/TestMethod")); @@ -1488,7 +1536,7 @@ TEST_F(MessageSizeParserTest, InvalidMaxRequestMessageBytes) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG @@ -1509,7 +1557,7 @@ TEST_F(MessageSizeParserTest, InvalidMaxResponseMessageBytes) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); EXPECT_THAT(grpc_error_std_string(error), ::testing::ContainsRegex( "Service config parsing error" CHILD_ERROR_TAG diff --git a/test/core/ext/filters/rbac/rbac_service_config_parser_test.cc b/test/core/ext/filters/rbac/rbac_service_config_parser_test.cc index 73c6fc31c1f..9ab1102c4dc 100644 --- a/test/core/ext/filters/rbac/rbac_service_config_parser_test.cc +++ b/test/core/ext/filters/rbac/rbac_service_config_parser_test.cc @@ -18,7 +18,7 @@ #include <gmock/gmock.h> #include <gtest/gtest.h> -#include "src/core/lib/service_config/service_config.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "test/core/util/test_config.h" // A regular expression to enter referenced or child errors. @@ -48,7 +48,7 @@ TEST(RbacServiceConfigParsingTest, EmptyRbacPolicy) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); @@ -76,7 +76,7 @@ TEST(RbacServiceConfigParsingTest, MissingChannelArg) { " } ]\n" "}"; grpc_error_handle error = GRPC_ERROR_NONE; - auto svc_cfg = ServiceConfig::Create(nullptr, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(nullptr, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); @@ -101,7 +101,7 @@ TEST(RbacServiceConfigParsingTest, EmptyRbacPolicyArray) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); @@ -126,7 +126,7 @@ TEST(RbacServiceConfigParsingTest, MultipleRbacPolicies) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); @@ -156,7 +156,7 @@ TEST(RbacServiceConfigParsingTest, BadRbacPolicyType) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex("Rbac parser" CHILD_ERROR_TAG @@ -178,7 +178,7 @@ TEST(RbacServiceConfigParsingTest, BadRulesType) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex("Rbac parser" CHILD_ERROR_TAG @@ -206,7 +206,7 @@ TEST(RbacServiceConfigParsingTest, BadActionAndPolicyType) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex("Rbac parser" CHILD_ERROR_TAG @@ -238,7 +238,7 @@ TEST(RbacServiceConfigParsingTest, MissingPermissionAndPrincipals) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex("Rbac parser" CHILD_ERROR_TAG @@ -273,7 +273,7 @@ TEST(RbacServiceConfigParsingTest, EmptyPrincipalAndPermission) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex( @@ -332,7 +332,7 @@ TEST(RbacServiceConfigParsingTest, VariousPermissionsAndPrincipalsTypes) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); @@ -391,7 +391,7 @@ TEST(RbacServiceConfigParsingTest, VariousPermissionsAndPrincipalsBadTypes) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex( @@ -477,7 +477,7 @@ TEST(RbacServiceConfigParsingTest, HeaderMatcherVariousTypes) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); @@ -522,7 +522,7 @@ TEST(RbacServiceConfigParsingTest, HeaderMatcherBadTypes) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex( @@ -578,7 +578,7 @@ TEST(RbacServiceConfigParsingTest, StringMatcherVariousTypes) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); const auto* vector_ptr = svc_cfg->GetMethodParsedConfigVector(grpc_empty_slice()); @@ -621,7 +621,7 @@ TEST(RbacServiceConfigParsingTest, StringMatcherBadTypes) { grpc_arg arg = grpc_channel_arg_integer_create( const_cast<char*>(GRPC_ARG_PARSE_RBAC_METHOD_CONFIG), 1); grpc_channel_args args = {1, &arg}; - auto svc_cfg = ServiceConfig::Create(&args, test_json, &error); + auto svc_cfg = ServiceConfigImpl::Create(&args, test_json, &error); EXPECT_THAT( grpc_error_std_string(error), ::testing::ContainsRegex("Rbac parser" CHILD_ERROR_TAG diff --git a/test/cpp/client/client_channel_stress_test.cc b/test/cpp/client/client_channel_stress_test.cc index ba251f0efbb..bcda12099f3 100644 --- a/test/cpp/client/client_channel_stress_test.cc +++ b/test/cpp/client/client_channel_stress_test.cc @@ -45,6 +45,7 @@ #include "src/core/lib/gprpp/thd.h" #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/resolver/server_address.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/proto/grpc/lb/v1/load_balancer.grpc.pb.h" #include "src/proto/grpc/testing/echo.grpc.pb.h" #include "test/core/util/port.h" @@ -241,7 +242,7 @@ class ClientChannelStressTest { const std::vector<AddressData>& balancer_address_data) { grpc_core::Resolver::Result result; grpc_error_handle error = GRPC_ERROR_NONE; - result.service_config = grpc_core::ServiceConfig::Create( + result.service_config = grpc_core::ServiceConfigImpl::Create( nullptr, "{\"loadBalancingConfig\":[{\"grpclb\":{}}]}", &error); GPR_ASSERT(error == GRPC_ERROR_NONE); grpc_core::ServerAddressList balancer_addresses = diff --git a/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc b/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc index 644bf876770..097ff61ae99 100644 --- a/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc +++ b/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc @@ -44,6 +44,7 @@ #include "src/core/lib/gprpp/thd.h" #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/resolver/server_address.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "test/core/util/port.h" #include "test/core/util/test_config.h" @@ -68,7 +69,7 @@ void TryConnectAndDestroy() { addresses.emplace_back(address.addr, address.len, nullptr); grpc_core::Resolver::Result lb_address_result; grpc_error_handle error = GRPC_ERROR_NONE; - lb_address_result.service_config = grpc_core::ServiceConfig::Create( + lb_address_result.service_config = grpc_core::ServiceConfigImpl::Create( nullptr, "{\"loadBalancingConfig\":[{\"grpclb\":{}}]}", &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); grpc_arg arg = grpc_core::CreateGrpclbBalancerAddressesArg(&addresses); diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index d96fac13a76..8b2085f7785 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/test/cpp/end2end/client_lb_end2end_test.cc @@ -54,6 +54,7 @@ #include "src/core/lib/resolver/server_address.h" #include "src/core/lib/security/credentials/fake/fake_credentials.h" #include "src/core/lib/service_config/service_config.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/cpp/client/secure_credentials.h" #include "src/cpp/server/secure_server_credentials.h" #include "src/proto/grpc/testing/echo.grpc.pb.h" @@ -213,7 +214,7 @@ class FakeResolverResponseGeneratorWrapper { } if (service_config_json != nullptr) { grpc_error_handle error = GRPC_ERROR_NONE; - result.service_config = grpc_core::ServiceConfig::Create( + result.service_config = grpc_core::ServiceConfigImpl::Create( nullptr, service_config_json, &error); GPR_ASSERT(*result.service_config != nullptr); } diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc index dd998badfb8..9e358299fb0 100644 --- a/test/cpp/end2end/grpclb_end2end_test.cc +++ b/test/cpp/end2end/grpclb_end2end_test.cc @@ -50,7 +50,7 @@ #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/resolver/server_address.h" #include "src/core/lib/security/credentials/fake/fake_credentials.h" -#include "src/core/lib/service_config/service_config.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/cpp/client/secure_credentials.h" #include "src/cpp/server/secure_server_credentials.h" #include "src/proto/grpc/lb/v1/load_balancer.grpc.pb.h" @@ -531,8 +531,8 @@ class GrpclbEnd2endTest : public ::testing::Test { result.addresses = CreateLbAddressesFromAddressDataList(backend_address_data); grpc_error_handle error = GRPC_ERROR_NONE; - result.service_config = - grpc_core::ServiceConfig::Create(nullptr, service_config_json, &error); + result.service_config = grpc_core::ServiceConfigImpl::Create( + nullptr, service_config_json, &error); GPR_ASSERT(error == GRPC_ERROR_NONE); grpc_core::ServerAddressList balancer_addresses = CreateLbAddressesFromAddressDataList(balancer_address_data); diff --git a/test/cpp/end2end/rls_end2end_test.cc b/test/cpp/end2end/rls_end2end_test.cc index b26b953be00..10bf07b92a6 100644 --- a/test/cpp/end2end/rls_end2end_test.cc +++ b/test/cpp/end2end/rls_end2end_test.cc @@ -47,6 +47,7 @@ #include "src/core/lib/gprpp/host_port.h" #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/security/credentials/fake/fake_credentials.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/core/lib/uri/uri_parser.h" #include "src/cpp/client/secure_credentials.h" #include "src/cpp/server/secure_server_credentials.h" @@ -230,7 +231,7 @@ class FakeResolverResponseGeneratorWrapper { absl::string_view service_config_json) { grpc_core::Resolver::Result result; grpc_error_handle error = GRPC_ERROR_NONE; - result.service_config = grpc_core::ServiceConfig::Create( + result.service_config = grpc_core::ServiceConfigImpl::Create( result.args, service_config_json, &error); EXPECT_EQ(error, GRPC_ERROR_NONE) << "JSON: " << service_config_json diff --git a/test/cpp/end2end/service_config_end2end_test.cc b/test/cpp/end2end/service_config_end2end_test.cc index 0f5cee0fdd3..13507ffd2bc 100644 --- a/test/cpp/end2end/service_config_end2end_test.cc +++ b/test/cpp/end2end/service_config_end2end_test.cc @@ -55,6 +55,7 @@ #include "src/core/lib/iomgr/tcp_client.h" #include "src/core/lib/resolver/server_address.h" #include "src/core/lib/security/credentials/fake/fake_credentials.h" +#include "src/core/lib/service_config/service_config_impl.h" #include "src/core/lib/transport/error_utils.h" #include "src/cpp/client/secure_credentials.h" #include "src/cpp/server/secure_server_credentials.h" @@ -199,7 +200,7 @@ class ServiceConfigEnd2endTest : public ::testing::Test { grpc_core::Resolver::Result result = BuildFakeResults(ports); grpc_error_handle error = GRPC_ERROR_NONE; result.service_config = - grpc_core::ServiceConfig::Create(nullptr, "{}", &error); + grpc_core::ServiceConfigImpl::Create(nullptr, "{}", &error); ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error); response_generator_->SetResponse(result); } @@ -218,7 +219,7 @@ class ServiceConfigEnd2endTest : public ::testing::Test { grpc_core::Resolver::Result result = BuildFakeResults(ports); grpc_error_handle error = GRPC_ERROR_NONE; result.service_config = - grpc_core::ServiceConfig::Create(nullptr, svc_cfg, &error); + grpc_core::ServiceConfigImpl::Create(nullptr, svc_cfg, &error); if (error != GRPC_ERROR_NONE) { result.service_config = grpc_error_to_absl_status(error); GRPC_ERROR_UNREF(error); diff --git a/test/cpp/naming/resolver_component_test.cc b/test/cpp/naming/resolver_component_test.cc index 4991b408395..a3bc02ad952 100644 --- a/test/cpp/naming/resolver_component_test.cc +++ b/test/cpp/naming/resolver_component_test.cc @@ -497,13 +497,12 @@ class CheckingResultHandler : public ResultHandler { if (!result.service_config.ok()) { CheckServiceConfigResultLocked(nullptr, result.service_config.status(), args); + } else if (*result.service_config == nullptr) { + CheckServiceConfigResultLocked(nullptr, absl::OkStatus(), args); } else { - const char* service_config_json = - *result.service_config == nullptr - ? nullptr - : (*result.service_config)->json_string().c_str(); - CheckServiceConfigResultLocked(service_config_json, absl::OkStatus(), - args); + CheckServiceConfigResultLocked( + std::string((*result.service_config)->json_string()).c_str(), + absl::OkStatus(), args); } if (args->expected_service_config_string.empty()) { CheckLBPolicyResultLocked(result.args, args); diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 077db1e6f08..9da0d5e0967 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2326,9 +2326,10 @@ src/core/lib/security/transport/tsi_error.cc \ src/core/lib/security/transport/tsi_error.h \ src/core/lib/security/util/json_util.cc \ src/core/lib/security/util/json_util.h \ -src/core/lib/service_config/service_config.cc \ src/core/lib/service_config/service_config.h \ src/core/lib/service_config/service_config_call_data.h \ +src/core/lib/service_config/service_config_impl.cc \ +src/core/lib/service_config/service_config_impl.h \ src/core/lib/service_config/service_config_parser.cc \ src/core/lib/service_config/service_config_parser.h \ src/core/lib/slice/b64.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index eba5b9d9363..daa66bf68ad 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -2121,9 +2121,10 @@ src/core/lib/security/transport/tsi_error.cc \ src/core/lib/security/transport/tsi_error.h \ src/core/lib/security/util/json_util.cc \ src/core/lib/security/util/json_util.h \ -src/core/lib/service_config/service_config.cc \ src/core/lib/service_config/service_config.h \ src/core/lib/service_config/service_config_call_data.h \ +src/core/lib/service_config/service_config_impl.cc \ +src/core/lib/service_config/service_config_impl.h \ src/core/lib/service_config/service_config_parser.cc \ src/core/lib/service_config/service_config_parser.h \ src/core/lib/slice/b64.cc \