diff --git a/CMakeLists.txt b/CMakeLists.txt index d16019a0c9e..dc9c2601ad3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1726,7 +1726,6 @@ add_library(grpc src/core/ext/filters/message_size/message_size_filter.cc src/core/ext/filters/rbac/rbac_filter.cc src/core/ext/filters/rbac/rbac_service_config_parser.cc - src/core/ext/filters/server_config_selector/server_config_selector.cc src/core/ext/filters/server_config_selector/server_config_selector_filter.cc src/core/ext/transport/chttp2/alpn/alpn.cc src/core/ext/transport/chttp2/client/chttp2_connector.cc diff --git a/Makefile b/Makefile index e3f7b2656b4..9581367dd6c 100644 --- a/Makefile +++ b/Makefile @@ -1028,7 +1028,6 @@ LIBGRPC_SRC = \ src/core/ext/filters/message_size/message_size_filter.cc \ src/core/ext/filters/rbac/rbac_filter.cc \ src/core/ext/filters/rbac/rbac_service_config_parser.cc \ - src/core/ext/filters/server_config_selector/server_config_selector.cc \ src/core/ext/filters/server_config_selector/server_config_selector_filter.cc \ src/core/ext/transport/chttp2/alpn/alpn.cc \ src/core/ext/transport/chttp2/client/chttp2_connector.cc \ @@ -2907,7 +2906,6 @@ src/core/ext/filters/client_channel/resolver/google_c2p/google_c2p_resolver.cc: src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc: $(OPENSSL_DEP) src/core/ext/filters/rbac/rbac_filter.cc: $(OPENSSL_DEP) src/core/ext/filters/rbac/rbac_service_config_parser.cc: $(OPENSSL_DEP) -src/core/ext/filters/server_config_selector/server_config_selector.cc: $(OPENSSL_DEP) src/core/ext/filters/server_config_selector/server_config_selector_filter.cc: $(OPENSSL_DEP) src/core/ext/transport/chttp2/alpn/alpn.cc: $(OPENSSL_DEP) src/core/ext/upb-generated/envoy/admin/v3/certs.upb.c: $(OPENSSL_DEP) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 5aa3b5fa327..ac84623aa2a 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -1114,7 +1114,6 @@ libs: - src/core/ext/filters/message_size/message_size_filter.cc - src/core/ext/filters/rbac/rbac_filter.cc - src/core/ext/filters/rbac/rbac_service_config_parser.cc - - src/core/ext/filters/server_config_selector/server_config_selector.cc - src/core/ext/filters/server_config_selector/server_config_selector_filter.cc - src/core/ext/transport/chttp2/alpn/alpn.cc - src/core/ext/transport/chttp2/client/chttp2_connector.cc diff --git a/config.m4 b/config.m4 index ec070b69f1c..6cd04edb730 100644 --- a/config.m4 +++ b/config.m4 @@ -110,7 +110,6 @@ if test "$PHP_GRPC" != "no"; then src/core/ext/filters/message_size/message_size_filter.cc \ src/core/ext/filters/rbac/rbac_filter.cc \ src/core/ext/filters/rbac/rbac_service_config_parser.cc \ - src/core/ext/filters/server_config_selector/server_config_selector.cc \ src/core/ext/filters/server_config_selector/server_config_selector_filter.cc \ src/core/ext/transport/chttp2/alpn/alpn.cc \ src/core/ext/transport/chttp2/client/chttp2_connector.cc \ diff --git a/config.w32 b/config.w32 index bd2e4d76f2a..d23443c7ce6 100644 --- a/config.w32 +++ b/config.w32 @@ -76,7 +76,6 @@ if (PHP_GRPC != "no") { "src\\core\\ext\\filters\\message_size\\message_size_filter.cc " + "src\\core\\ext\\filters\\rbac\\rbac_filter.cc " + "src\\core\\ext\\filters\\rbac\\rbac_service_config_parser.cc " + - "src\\core\\ext\\filters\\server_config_selector\\server_config_selector.cc " + "src\\core\\ext\\filters\\server_config_selector\\server_config_selector_filter.cc " + "src\\core\\ext\\transport\\chttp2\\alpn\\alpn.cc " + "src\\core\\ext\\transport\\chttp2\\client\\chttp2_connector.cc " + diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index caad84fb465..b261e98fdd2 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -322,7 +322,6 @@ Pod::Spec.new do |s| 'src/core/ext/filters/rbac/rbac_filter.h', 'src/core/ext/filters/rbac/rbac_service_config_parser.cc', 'src/core/ext/filters/rbac/rbac_service_config_parser.h', - 'src/core/ext/filters/server_config_selector/server_config_selector.cc', 'src/core/ext/filters/server_config_selector/server_config_selector.h', 'src/core/ext/filters/server_config_selector/server_config_selector_filter.cc', 'src/core/ext/filters/server_config_selector/server_config_selector_filter.h', diff --git a/grpc.gemspec b/grpc.gemspec index 4a5dc2ba3e1..a95bc561652 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -233,7 +233,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/filters/rbac/rbac_filter.h ) s.files += %w( src/core/ext/filters/rbac/rbac_service_config_parser.cc ) s.files += %w( src/core/ext/filters/rbac/rbac_service_config_parser.h ) - s.files += %w( src/core/ext/filters/server_config_selector/server_config_selector.cc ) s.files += %w( src/core/ext/filters/server_config_selector/server_config_selector.h ) s.files += %w( src/core/ext/filters/server_config_selector/server_config_selector_filter.cc ) s.files += %w( src/core/ext/filters/server_config_selector/server_config_selector_filter.h ) diff --git a/grpc.gyp b/grpc.gyp index 73dabcb0cd7..dc94ac864aa 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -440,7 +440,6 @@ 'src/core/ext/filters/message_size/message_size_filter.cc', 'src/core/ext/filters/rbac/rbac_filter.cc', 'src/core/ext/filters/rbac/rbac_service_config_parser.cc', - 'src/core/ext/filters/server_config_selector/server_config_selector.cc', 'src/core/ext/filters/server_config_selector/server_config_selector_filter.cc', 'src/core/ext/transport/chttp2/alpn/alpn.cc', 'src/core/ext/transport/chttp2/client/chttp2_connector.cc', diff --git a/package.xml b/package.xml index 99ba22450ef..d2eea2fb4a0 100644 --- a/package.xml +++ b/package.xml @@ -215,7 +215,6 @@ - diff --git a/src/core/BUILD b/src/core/BUILD index 323dae2c7a6..22094928779 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -2327,9 +2327,6 @@ grpc_cc_library( grpc_cc_library( name = "grpc_server_config_selector", - srcs = [ - "ext/filters/server_config_selector/server_config_selector.cc", - ], hdrs = [ "ext/filters/server_config_selector/server_config_selector.h", ], @@ -2339,7 +2336,6 @@ grpc_cc_library( ], language = "c++", deps = [ - "channel_args", "dual_ref_counted", "error", "grpc_service_config", diff --git a/src/core/ext/filters/server_config_selector/server_config_selector.cc b/src/core/ext/filters/server_config_selector/server_config_selector.cc deleted file mode 100644 index 96105879e86..00000000000 --- a/src/core/ext/filters/server_config_selector/server_config_selector.cc +++ /dev/null @@ -1,62 +0,0 @@ -// -// Copyright 2021 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. -// - -#include - -#include "src/core/ext/filters/server_config_selector/server_config_selector.h" - -#include "src/core/lib/channel/channel_args.h" -#include "src/core/lib/gpr/useful.h" - -namespace grpc_core { -namespace { - -void* ServerConfigSelectorProviderArgCopy(void* p) { - ServerConfigSelectorProvider* arg = - static_cast(p); - return arg->Ref().release(); -} - -void ServerConfigSelectorProviderArgDestroy(void* p) { - ServerConfigSelectorProvider* arg = - static_cast(p); - arg->Unref(); -} - -int ServerConfigSelectorProviderArgCmp(void* p, void* q) { - return QsortCompare(p, q); -} - -const grpc_arg_pointer_vtable kChannelArgVtable = { - ServerConfigSelectorProviderArgCopy, ServerConfigSelectorProviderArgDestroy, - ServerConfigSelectorProviderArgCmp}; - -const char* kServerConfigSelectorProviderChannelArgName = - "grpc.internal.server_config_selector_provider"; - -} // namespace - -grpc_arg ServerConfigSelectorProvider::MakeChannelArg() const { - return grpc_channel_arg_pointer_create( - const_cast(kServerConfigSelectorProviderChannelArgName), - const_cast(this), &kChannelArgVtable); -} - -absl::string_view ServerConfigSelectorProvider::ChannelArgName() { - return kServerConfigSelectorProviderChannelArgName; -} - -} // namespace grpc_core diff --git a/src/core/ext/filters/server_config_selector/server_config_selector.h b/src/core/ext/filters/server_config_selector/server_config_selector.h index 3bcd90c0923..d68ee2e0a3e 100644 --- a/src/core/ext/filters/server_config_selector/server_config_selector.h +++ b/src/core/ext/filters/server_config_selector/server_config_selector.h @@ -43,14 +43,15 @@ class ServerConfigSelector : public RefCounted { public: // Configuration to apply to an incoming call struct CallConfig { - grpc_error_handle error; const ServiceConfigParser::ParsedConfigVector* method_configs = nullptr; RefCountedPtr service_config; }; ~ServerConfigSelector() override = default; + // Returns the CallConfig to apply to a call based on the incoming \a metadata - virtual CallConfig GetCallConfig(grpc_metadata_batch* metadata) = 0; + virtual absl::StatusOr GetCallConfig( + grpc_metadata_batch* metadata) = 0; }; // ServerConfigSelectorProvider allows for subscribers to watch for updates on @@ -71,13 +72,13 @@ class ServerConfigSelectorProvider std::unique_ptr watcher) = 0; virtual void CancelWatch() = 0; - static absl::string_view ChannelArgName(); + static absl::string_view ChannelArgName() { + return "grpc.internal.server_config_selector_provider"; + } static int ChannelArgsCompare(const ServerConfigSelectorProvider* a, const ServerConfigSelectorProvider* b) { return QsortCompare(a, b); } - - grpc_arg MakeChannelArg() const; }; } // namespace grpc_core diff --git a/src/core/ext/filters/server_config_selector/server_config_selector_filter.cc b/src/core/ext/filters/server_config_selector/server_config_selector_filter.cc index bed20e7a984..0f17ff9fdcf 100644 --- a/src/core/ext/filters/server_config_selector/server_config_selector_filter.cc +++ b/src/core/ext/filters/server_config_selector/server_config_selector_filter.cc @@ -135,15 +135,15 @@ ArenaPromise ServerConfigSelectorFilter::MakeCallPromise( if (!sel.ok()) return Immediate(ServerMetadataFromStatus(sel.status())); auto call_config = sel.value()->GetCallConfig(call_args.client_initial_metadata.get()); - if (!call_config.error.ok()) { + if (!call_config.ok()) { auto r = Immediate(ServerMetadataFromStatus( - absl::UnavailableError(StatusToString(call_config.error)))); + absl::UnavailableError(StatusToString(call_config.status())))); return std::move(r); } auto& ctx = GetContext< grpc_call_context_element>()[GRPC_CONTEXT_SERVICE_CONFIG_CALL_DATA]; ctx.value = GetContext()->New( - std::move(call_config.service_config), call_config.method_configs, + std::move(call_config->service_config), call_config->method_configs, ServiceConfigCallData::CallAttributes{}); ctx.destroy = [](void* p) { static_cast(p)->~ServiceConfigCallData(); diff --git a/src/core/ext/xds/xds_server_config_fetcher.cc b/src/core/ext/xds/xds_server_config_fetcher.cc index 9ca51ba224a..2712254a2eb 100644 --- a/src/core/ext/xds/xds_server_config_fetcher.cc +++ b/src/core/ext/xds/xds_server_config_fetcher.cc @@ -319,7 +319,8 @@ class XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager:: http_filters); ~XdsServerConfigSelector() override = default; - CallConfig GetCallConfig(grpc_metadata_batch* metadata) override; + absl::StatusOr GetCallConfig( + grpc_metadata_batch* metadata) override; private: struct VirtualHost { @@ -1188,30 +1189,26 @@ XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager:: return config_selector; } -ServerConfigSelector::CallConfig XdsServerConfigFetcher::ListenerWatcher:: - FilterChainMatchManager::XdsServerConfigSelector::GetCallConfig( - grpc_metadata_batch* metadata) { +absl::StatusOr +XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager:: + XdsServerConfigSelector::GetCallConfig(grpc_metadata_batch* metadata) { CallConfig call_config; if (metadata->get_pointer(HttpPathMetadata()) == nullptr) { - call_config.error = GRPC_ERROR_CREATE("No path found"); - return call_config; + return absl::InternalError("no path found"); } absl::string_view path = metadata->get_pointer(HttpPathMetadata())->as_string_view(); if (metadata->get_pointer(HttpAuthorityMetadata()) == nullptr) { - call_config.error = GRPC_ERROR_CREATE("No authority found"); - return call_config; + return absl::InternalError("no authority found"); } absl::string_view authority = metadata->get_pointer(HttpAuthorityMetadata())->as_string_view(); auto vhost_index = XdsRouting::FindVirtualHostForDomain( VirtualHostListIterator(&virtual_hosts_), authority); if (!vhost_index.has_value()) { - call_config.error = grpc_error_set_int( - GRPC_ERROR_CREATE(absl::StrCat("could not find VirtualHost for ", - authority, " in RouteConfiguration")), - StatusIntProperty::kRpcStatus, GRPC_STATUS_UNAVAILABLE); - return call_config; + return absl::UnavailableError( + absl::StrCat("could not find VirtualHost for ", authority, + " in RouteConfiguration")); } auto& virtual_host = virtual_hosts_[vhost_index.value()]; auto route_index = XdsRouting::GetRouteForRequest( @@ -1220,10 +1217,7 @@ ServerConfigSelector::CallConfig XdsServerConfigFetcher::ListenerWatcher:: auto& route = virtual_host.routes[route_index.value()]; // Found the matching route if (route.unsupported_action) { - call_config.error = grpc_error_set_int( - GRPC_ERROR_CREATE("Matching route has unsupported action"), - StatusIntProperty::kRpcStatus, GRPC_STATUS_UNAVAILABLE); - return call_config; + return absl::UnavailableError("matching route has unsupported action"); } if (route.method_config != nullptr) { call_config.method_configs = @@ -1232,10 +1226,7 @@ ServerConfigSelector::CallConfig XdsServerConfigFetcher::ListenerWatcher:: } return call_config; } - call_config.error = grpc_error_set_int(GRPC_ERROR_CREATE("No route matched"), - StatusIntProperty::kRpcStatus, - GRPC_STATUS_UNAVAILABLE); - return call_config; + return absl::UnavailableError("no route matched"); } // diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 469013ec22d..bb788181bdf 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -85,7 +85,6 @@ CORE_SOURCE_FILES = [ 'src/core/ext/filters/message_size/message_size_filter.cc', 'src/core/ext/filters/rbac/rbac_filter.cc', 'src/core/ext/filters/rbac/rbac_service_config_parser.cc', - 'src/core/ext/filters/server_config_selector/server_config_selector.cc', 'src/core/ext/filters/server_config_selector/server_config_selector_filter.cc', 'src/core/ext/transport/chttp2/alpn/alpn.cc', 'src/core/ext/transport/chttp2/client/chttp2_connector.cc', diff --git a/test/core/server_config_selector/server_config_selector_test.cc b/test/core/server_config_selector/server_config_selector_test.cc index df0b67f1ace..4ed25ae504e 100644 --- a/test/core/server_config_selector/server_config_selector_test.cc +++ b/test/core/server_config_selector/server_config_selector_test.cc @@ -26,16 +26,13 @@ #include "src/core/lib/channel/channel_args.h" #include "test/core/util/test_config.h" -namespace grpc { +namespace grpc_core { namespace testing { namespace { -using grpc_core::ServerConfigSelector; -using grpc_core::ServerConfigSelectorProvider; - class TestServerConfigSelectorProvider : public ServerConfigSelectorProvider { - absl::StatusOr> Watch( - std::unique_ptr /* watcher */) override { + absl::StatusOr> Watch( + std::unique_ptr /*watcher*/) override { return absl::UnavailableError("Test ServerConfigSelector"); } @@ -48,33 +45,24 @@ class TestServerConfigSelectorProvider : public ServerConfigSelectorProvider { // and destroyed TEST(ServerConfigSelectorProviderTest, CopyChannelArgs) { auto server_config_selector_provider = - grpc_core::MakeRefCounted(); - grpc_arg arg = server_config_selector_provider->MakeChannelArg(); - grpc_channel_args* args = grpc_channel_args_copy_and_add(nullptr, &arg, 1); + MakeRefCounted(); + auto args = ChannelArgs().SetObject(server_config_selector_provider); EXPECT_EQ(server_config_selector_provider, - grpc_core::ChannelArgs::FromC(args) - .GetObject()); - grpc_channel_args_destroy(args); + args.GetObject()); } // Test compare on channel args with the same ServerConfigSelectorProvider TEST(ServerConfigSelectorProviderTest, ChannelArgsCompare) { auto server_config_selector_provider = - grpc_core::MakeRefCounted(); - grpc_arg arg = server_config_selector_provider->MakeChannelArg(); - grpc_channel_args* args = grpc_channel_args_copy_and_add(nullptr, &arg, 1); - grpc_channel_args* new_args = grpc_channel_args_copy(args); - EXPECT_EQ(grpc_core::ChannelArgs::FromC(new_args) - .GetObject(), - grpc_core::ChannelArgs::FromC(args) - .GetObject()); - grpc_channel_args_destroy(args); - grpc_channel_args_destroy(new_args); + MakeRefCounted(); + auto args = ChannelArgs().SetObject(server_config_selector_provider); + auto args2 = ChannelArgs().SetObject(server_config_selector_provider); + EXPECT_EQ(args, args2); } } // namespace } // namespace testing -} // namespace grpc +} // namespace grpc_core int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 4854faa51db..6bb1123f9cc 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1192,7 +1192,6 @@ src/core/ext/filters/rbac/rbac_filter.cc \ src/core/ext/filters/rbac/rbac_filter.h \ src/core/ext/filters/rbac/rbac_service_config_parser.cc \ src/core/ext/filters/rbac/rbac_service_config_parser.h \ -src/core/ext/filters/server_config_selector/server_config_selector.cc \ src/core/ext/filters/server_config_selector/server_config_selector.h \ src/core/ext/filters/server_config_selector/server_config_selector_filter.cc \ src/core/ext/filters/server_config_selector/server_config_selector_filter.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index f3e55c5cdc2..2e9ea2d7dcf 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1000,7 +1000,6 @@ src/core/ext/filters/rbac/rbac_filter.cc \ src/core/ext/filters/rbac/rbac_filter.h \ src/core/ext/filters/rbac/rbac_service_config_parser.cc \ src/core/ext/filters/rbac/rbac_service_config_parser.h \ -src/core/ext/filters/server_config_selector/server_config_selector.cc \ src/core/ext/filters/server_config_selector/server_config_selector.h \ src/core/ext/filters/server_config_selector/server_config_selector_filter.cc \ src/core/ext/filters/server_config_selector/server_config_selector_filter.h \