xDS: change remaining registries to be non-global (#31293)

* json_object_loader: refactor ErrorList into its own library

* fix observability_config_test

* generate_projects

* iwyu

* WIP

* xDS: make HTTP filter registry non-global

* make cluster specifier plugin registry non-global

* fix test

* clang-format

* general-purpose utility for validation xDS extensions

* plumb XdsExtension into HTTP filters and ClusterSpecifiers

* clang-format

* fix tests

* iwyu

* fix build with old compilers

* finish plumbing ValidationErrors into Listener, including HTTP filters

* fix tests

* WIP

* use absl::variant for listener type

* use absl::variant for RDS name vs. inline RouteConfig

* first passing test

* adding more tests

* xDS RBAC: remove env var protection

* fix federation server test

* add another test

* fix sanity

* first pass of client-side validation, except for HTTP filter configs

* minor cleanup and TODO

* clean up HTTP filter registration

* fix unused param

* fix tests

* WIP on xds_http_filters_test

* clang-format

* add tests for fault injection filter

* RBAC filter tests

* Automated change: Fix sanity tests

* fix sanity

* iwyu

* fix asan problem

* started adding server-side listener tests

* DownstreamTlsContext tests

* update filter chain duplicate detection code

* clang-format

* migrate remaining tests from e2e tests and remove duplicates

* parameterized HCM tests

* clang-format

* generate_projects

* iwyu

* remove unnecessary dependency

* remove another unnecessary dep

* remove more unneeded deps

* remove no_op_http_filter

* add death test for duplicate HTTP filter registration

* clang-format

* remove unnecessary StaticConfigName() method from Router filter

* clang-format

* iwyu

Co-authored-by: markdroth <markdroth@users.noreply.github.com>
pull/31469/head
Mark D. Roth 2 years ago committed by GitHub
parent 9143627664
commit 26f74399b0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 7
      src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc
  2. 11
      src/core/ext/xds/xds_bootstrap_grpc.h
  3. 2
      src/core/ext/xds/xds_client.cc
  4. 9
      src/core/ext/xds/xds_client_grpc.cc
  5. 3
      src/core/ext/xds/xds_cluster.h
  6. 47
      src/core/ext/xds/xds_cluster_specifier_plugin.cc
  7. 35
      src/core/ext/xds/xds_cluster_specifier_plugin.h
  8. 3
      src/core/ext/xds/xds_endpoint.h
  9. 50
      src/core/ext/xds/xds_http_filters.cc
  10. 34
      src/core/ext/xds/xds_http_filters.h
  11. 8
      src/core/ext/xds/xds_listener.cc
  12. 10
      src/core/ext/xds/xds_listener.h
  13. 3
      src/core/ext/xds/xds_resource_type.h
  14. 10
      src/core/ext/xds/xds_route_config.cc
  15. 10
      src/core/ext/xds/xds_route_config.h
  16. 3
      src/core/ext/xds/xds_routing.cc
  17. 2
      src/core/ext/xds/xds_routing.h
  18. 42
      src/core/ext/xds/xds_server_config_fetcher.cc
  19. 2
      test/core/xds/xds_client_test.cc
  20. 20
      test/core/xds/xds_http_filters_test.cc

@ -523,11 +523,14 @@ XdsResolver::XdsConfigSelector::XdsConfigSelector(
} }
} }
// Populate filter list. // Populate filter list.
const auto& http_filter_registry =
static_cast<const GrpcXdsBootstrap&>(resolver_->xds_client_->bootstrap())
.http_filter_registry();
for (const auto& http_filter : resolver_->current_listener_.http_filters) { for (const auto& http_filter : resolver_->current_listener_.http_filters) {
// Find filter. This is guaranteed to succeed, because it's checked // Find filter. This is guaranteed to succeed, because it's checked
// at config validation time in the XdsApi code. // at config validation time in the XdsApi code.
const XdsHttpFilterImpl* filter_impl = const XdsHttpFilterImpl* filter_impl =
XdsHttpFilterRegistry::GetFilterForType( http_filter_registry.GetFilterForType(
http_filter.config.config_proto_type_name); http_filter.config.config_proto_type_name);
GPR_ASSERT(filter_impl != nullptr); GPR_ASSERT(filter_impl != nullptr);
// Add C-core filter to list. // Add C-core filter to list.
@ -600,6 +603,8 @@ XdsResolver::XdsConfigSelector::CreateMethodConfig(
} }
// Handle xDS HTTP filters. // Handle xDS HTTP filters.
auto result = XdsRouting::GeneratePerHTTPFilterConfigs( auto result = XdsRouting::GeneratePerHTTPFilterConfigs(
static_cast<const GrpcXdsBootstrap&>(resolver_->xds_client_->bootstrap())
.http_filter_registry(),
resolver_->current_listener_.http_filters, resolver_->current_listener_.http_filters,
resolver_->current_virtual_host_, route, cluster_weight, resolver_->current_virtual_host_, route, cluster_weight,
resolver_->args_); resolver_->args_);

@ -31,6 +31,8 @@
#include "src/core/ext/xds/certificate_provider_store.h" #include "src/core/ext/xds/certificate_provider_store.h"
#include "src/core/ext/xds/xds_bootstrap.h" #include "src/core/ext/xds/xds_bootstrap.h"
#include "src/core/ext/xds/xds_cluster_specifier_plugin.h"
#include "src/core/ext/xds/xds_http_filters.h"
#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/gprpp/validation_errors.h"
#include "src/core/lib/json/json.h" #include "src/core/lib/json/json.h"
#include "src/core/lib/json/json_args.h" #include "src/core/lib/json/json_args.h"
@ -148,6 +150,13 @@ class GrpcXdsBootstrap : public XdsBootstrap {
const { const {
return certificate_providers_; return certificate_providers_;
} }
const XdsHttpFilterRegistry& http_filter_registry() const {
return http_filter_registry_;
}
const XdsClusterSpecifierPluginRegistry& cluster_specifier_plugin_registry()
const {
return cluster_specifier_plugin_registry_;
}
// Exposed for testing purposes only. // Exposed for testing purposes only.
const std::map<std::string, GrpcAuthority>& authorities() const { const std::map<std::string, GrpcAuthority>& authorities() const {
@ -161,6 +170,8 @@ class GrpcXdsBootstrap : public XdsBootstrap {
std::string server_listener_resource_name_template_; std::string server_listener_resource_name_template_;
std::map<std::string, GrpcAuthority> authorities_; std::map<std::string, GrpcAuthority> authorities_;
CertificateProviderStore::PluginDefinitionMap certificate_providers_; CertificateProviderStore::PluginDefinitionMap certificate_providers_;
XdsHttpFilterRegistry http_filter_registry_;
XdsClusterSpecifierPluginRegistry cluster_specifier_plugin_registry_;
}; };
} // namespace grpc_core } // namespace grpc_core

@ -1641,7 +1641,7 @@ void XdsClient::MaybeRegisterResourceTypeLocked(
return; return;
} }
resource_types_.emplace(resource_type->type_url(), resource_type); resource_types_.emplace(resource_type->type_url(), resource_type);
resource_type->InitUpbSymtab(symtab_.ptr()); resource_type->InitUpbSymtab(this, symtab_.ptr());
} }
const XdsResourceType* XdsClient::GetResourceTypeLocked( const XdsResourceType* XdsClient::GetResourceTypeLocked(

@ -26,7 +26,6 @@
#include "absl/base/thread_annotations.h" #include "absl/base/thread_annotations.h"
#include "absl/status/status.h" #include "absl/status/status.h"
#include "absl/strings/string_view.h" #include "absl/strings/string_view.h"
#include "absl/synchronization/mutex.h"
#include "absl/types/optional.h" #include "absl/types/optional.h"
#include <grpc/grpc.h> #include <grpc/grpc.h>
@ -38,8 +37,6 @@
#include "src/core/ext/xds/xds_bootstrap.h" #include "src/core/ext/xds/xds_bootstrap.h"
#include "src/core/ext/xds/xds_bootstrap_grpc.h" #include "src/core/ext/xds/xds_bootstrap_grpc.h"
#include "src/core/ext/xds/xds_channel_args.h" #include "src/core/ext/xds/xds_channel_args.h"
#include "src/core/ext/xds/xds_cluster_specifier_plugin.h"
#include "src/core/ext/xds/xds_http_filters.h"
#include "src/core/ext/xds/xds_transport.h" #include "src/core/ext/xds/xds_transport.h"
#include "src/core/ext/xds/xds_transport_grpc.h" #include "src/core/ext/xds/xds_transport_grpc.h"
#include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_args.h"
@ -65,11 +62,7 @@ namespace grpc_core {
namespace { namespace {
Mutex* g_mu = [] { Mutex* g_mu = new Mutex;
XdsHttpFilterRegistry::Init();
XdsClusterSpecifierPluginRegistry::Init();
return new Mutex;
}();
const grpc_channel_args* g_channel_args ABSL_GUARDED_BY(*g_mu) = nullptr; const grpc_channel_args* g_channel_args ABSL_GUARDED_BY(*g_mu) = nullptr;
GrpcXdsClient* g_xds_client ABSL_GUARDED_BY(*g_mu) = nullptr; GrpcXdsClient* g_xds_client ABSL_GUARDED_BY(*g_mu) = nullptr;
char* g_fallback_bootstrap_config ABSL_GUARDED_BY(*g_mu) = nullptr; char* g_fallback_bootstrap_config ABSL_GUARDED_BY(*g_mu) = nullptr;

@ -36,6 +36,7 @@
#include "src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h" #include "src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.h"
#include "src/core/ext/xds/xds_bootstrap.h" #include "src/core/ext/xds/xds_bootstrap.h"
#include "src/core/ext/xds/xds_bootstrap_grpc.h" #include "src/core/ext/xds/xds_bootstrap_grpc.h"
#include "src/core/ext/xds/xds_client.h"
#include "src/core/ext/xds/xds_common_types.h" #include "src/core/ext/xds/xds_common_types.h"
#include "src/core/ext/xds/xds_resource_type.h" #include "src/core/ext/xds/xds_resource_type.h"
#include "src/core/ext/xds/xds_resource_type_impl.h" #include "src/core/ext/xds/xds_resource_type_impl.h"
@ -100,7 +101,7 @@ class XdsClusterResourceType
bool AllResourcesRequiredInSotW() const override { return true; } bool AllResourcesRequiredInSotW() const override { return true; }
void InitUpbSymtab(upb_DefPool* symtab) const override { void InitUpbSymtab(XdsClient*, upb_DefPool* symtab) const override {
envoy_config_cluster_v3_Cluster_getmsgdef(symtab); envoy_config_cluster_v3_Cluster_getmsgdef(symtab);
envoy_extensions_clusters_aggregate_v3_ClusterConfig_getmsgdef(symtab); envoy_extensions_clusters_aggregate_v3_ClusterConfig_getmsgdef(symtab);
envoy_extensions_transport_sockets_tls_v3_UpstreamTlsContext_getmsgdef( envoy_extensions_transport_sockets_tls_v3_UpstreamTlsContext_getmsgdef(

@ -41,6 +41,10 @@
namespace grpc_core { namespace grpc_core {
//
// XdsRouteLookupClusterSpecifierPlugin
//
const char* kXdsRouteLookupClusterSpecifierPluginConfigName = const char* kXdsRouteLookupClusterSpecifierPluginConfigName =
"grpc.lookup.v1.RouteLookupClusterSpecifier"; "grpc.lookup.v1.RouteLookupClusterSpecifier";
@ -114,41 +118,34 @@ XdsRouteLookupClusterSpecifierPlugin::GenerateLoadBalancingPolicyConfig(
return lb_policy_config.Dump(); return lb_policy_config.Dump();
} }
namespace { //
// XdsClusterSpecifierPluginRegistry
using PluginRegistryMap = //
std::map<absl::string_view, std::unique_ptr<XdsClusterSpecifierPluginImpl>>;
PluginRegistryMap* g_plugin_registry = nullptr; XdsClusterSpecifierPluginRegistry::XdsClusterSpecifierPluginRegistry() {
RegisterPlugin(std::make_unique<XdsRouteLookupClusterSpecifierPlugin>(),
kXdsRouteLookupClusterSpecifierPluginConfigName);
}
} // namespace void XdsClusterSpecifierPluginRegistry::RegisterPlugin(
std::unique_ptr<XdsClusterSpecifierPluginImpl> plugin,
absl::string_view config_proto_type_name) {
registry_[config_proto_type_name] = std::move(plugin);
}
const XdsClusterSpecifierPluginImpl* const XdsClusterSpecifierPluginImpl*
XdsClusterSpecifierPluginRegistry::GetPluginForType( XdsClusterSpecifierPluginRegistry::GetPluginForType(
absl::string_view config_proto_type_name) { absl::string_view config_proto_type_name) const {
auto it = g_plugin_registry->find(config_proto_type_name); auto it = registry_.find(config_proto_type_name);
if (it == g_plugin_registry->end()) return nullptr; if (it == registry_.end()) return nullptr;
return it->second.get(); return it->second.get();
} }
void XdsClusterSpecifierPluginRegistry::PopulateSymtab(upb_DefPool* symtab) { void XdsClusterSpecifierPluginRegistry::PopulateSymtab(
for (const auto& p : *g_plugin_registry) { upb_DefPool* symtab) const {
for (const auto& p : registry_) {
p.second->PopulateSymtab(symtab); p.second->PopulateSymtab(symtab);
} }
} }
void XdsClusterSpecifierPluginRegistry::RegisterPlugin(
std::unique_ptr<XdsClusterSpecifierPluginImpl> plugin,
absl::string_view config_proto_type_name) {
(*g_plugin_registry)[config_proto_type_name] = std::move(plugin);
}
void XdsClusterSpecifierPluginRegistry::Init() {
g_plugin_registry = new PluginRegistryMap;
RegisterPlugin(std::make_unique<XdsRouteLookupClusterSpecifierPlugin>(),
kXdsRouteLookupClusterSpecifierPluginConfigName);
}
void XdsClusterSpecifierPluginRegistry::Shutdown() { delete g_plugin_registry; }
} // namespace grpc_core } // namespace grpc_core

@ -19,8 +19,10 @@
#include <grpc/support/port_platform.h> #include <grpc/support/port_platform.h>
#include <map>
#include <memory> #include <memory>
#include <string> #include <string>
#include <utility>
#include "absl/status/statusor.h" #include "absl/status/statusor.h"
#include "absl/strings/string_view.h" #include "absl/strings/string_view.h"
@ -54,18 +56,35 @@ class XdsRouteLookupClusterSpecifierPlugin
class XdsClusterSpecifierPluginRegistry { class XdsClusterSpecifierPluginRegistry {
public: public:
static void RegisterPlugin( XdsClusterSpecifierPluginRegistry();
std::unique_ptr<XdsClusterSpecifierPluginImpl> plugin,
// Not copyable.
XdsClusterSpecifierPluginRegistry(const XdsClusterSpecifierPluginRegistry&) =
delete;
XdsClusterSpecifierPluginRegistry& operator=(
const XdsClusterSpecifierPluginRegistry&) = delete;
// Movable.
XdsClusterSpecifierPluginRegistry(
XdsClusterSpecifierPluginRegistry&& other) noexcept
: registry_(std::move(other.registry_)) {}
XdsClusterSpecifierPluginRegistry& operator=(
XdsClusterSpecifierPluginRegistry&& other) noexcept {
registry_ = std::move(other.registry_);
return *this;
}
void RegisterPlugin(std::unique_ptr<XdsClusterSpecifierPluginImpl> plugin,
absl::string_view config_proto_type_name); absl::string_view config_proto_type_name);
static void PopulateSymtab(upb_DefPool* symtab); void PopulateSymtab(upb_DefPool* symtab) const;
static const XdsClusterSpecifierPluginImpl* GetPluginForType( const XdsClusterSpecifierPluginImpl* GetPluginForType(
absl::string_view config_proto_type_name); absl::string_view config_proto_type_name) const;
// Global init and shutdown. private:
static void Init(); std::map<absl::string_view, std::unique_ptr<XdsClusterSpecifierPluginImpl>>
static void Shutdown(); registry_;
}; };
} // namespace grpc_core } // namespace grpc_core

@ -32,6 +32,7 @@
#include "envoy/config/endpoint/v3/endpoint.upbdefs.h" #include "envoy/config/endpoint/v3/endpoint.upbdefs.h"
#include "upb/def.h" #include "upb/def.h"
#include "src/core/ext/xds/xds_client.h"
#include "src/core/ext/xds/xds_client_stats.h" #include "src/core/ext/xds/xds_client_stats.h"
#include "src/core/ext/xds/xds_resource_type.h" #include "src/core/ext/xds/xds_resource_type.h"
#include "src/core/ext/xds/xds_resource_type_impl.h" #include "src/core/ext/xds/xds_resource_type_impl.h"
@ -128,7 +129,7 @@ class XdsEndpointResourceType
DecodeResult Decode(const XdsResourceType::DecodeContext& context, DecodeResult Decode(const XdsResourceType::DecodeContext& context,
absl::string_view serialized_resource) const override; absl::string_view serialized_resource) const override;
void InitUpbSymtab(upb_DefPool* symtab) const override { void InitUpbSymtab(XdsClient*, upb_DefPool* symtab) const override {
envoy_config_endpoint_v3_ClusterLoadAssignment_getmsgdef(symtab); envoy_config_endpoint_v3_ClusterLoadAssignment_getmsgdef(symtab);
} }
}; };

@ -81,54 +81,36 @@ XdsHttpRouterFilter::GenerateFilterConfigOverride(
// XdsHttpFilterRegistry // XdsHttpFilterRegistry
// //
namespace { XdsHttpFilterRegistry::XdsHttpFilterRegistry(bool register_builtins) {
if (register_builtins) {
using FilterOwnerList = std::vector<std::unique_ptr<XdsHttpFilterImpl>>; RegisterFilter(std::make_unique<XdsHttpRouterFilter>());
using FilterRegistryMap = std::map<absl::string_view, XdsHttpFilterImpl*>; RegisterFilter(std::make_unique<XdsHttpFaultFilter>());
RegisterFilter(std::make_unique<XdsHttpRbacFilter>());
FilterOwnerList* g_filters = nullptr; }
FilterRegistryMap* g_filter_registry = nullptr; }
} // namespace
void XdsHttpFilterRegistry::RegisterFilter( void XdsHttpFilterRegistry::RegisterFilter(
std::unique_ptr<XdsHttpFilterImpl> filter) { std::unique_ptr<XdsHttpFilterImpl> filter) {
GPR_ASSERT(g_filter_registry->emplace(filter->ConfigProtoName(), filter.get()) GPR_ASSERT(
.second); registry_map_.emplace(filter->ConfigProtoName(), filter.get()).second);
auto override_proto_name = filter->OverrideConfigProtoName(); auto override_proto_name = filter->OverrideConfigProtoName();
if (!override_proto_name.empty()) { if (!override_proto_name.empty()) {
GPR_ASSERT( GPR_ASSERT(registry_map_.emplace(override_proto_name, filter.get()).second);
g_filter_registry->emplace(override_proto_name, filter.get()).second);
} }
g_filters->push_back(std::move(filter)); owning_list_.push_back(std::move(filter));
} }
const XdsHttpFilterImpl* XdsHttpFilterRegistry::GetFilterForType( const XdsHttpFilterImpl* XdsHttpFilterRegistry::GetFilterForType(
absl::string_view proto_type_name) { absl::string_view proto_type_name) const {
auto it = g_filter_registry->find(proto_type_name); auto it = registry_map_.find(proto_type_name);
if (it == g_filter_registry->end()) return nullptr; if (it == registry_map_.end()) return nullptr;
return it->second; return it->second;
} }
void XdsHttpFilterRegistry::PopulateSymtab(upb_DefPool* symtab) { void XdsHttpFilterRegistry::PopulateSymtab(upb_DefPool* symtab) const {
for (const auto& filter : *g_filters) { for (const auto& filter : owning_list_) {
filter->PopulateSymtab(symtab); filter->PopulateSymtab(symtab);
} }
} }
void XdsHttpFilterRegistry::Init(bool register_builtins) {
g_filters = new FilterOwnerList;
g_filter_registry = new FilterRegistryMap;
if (register_builtins) {
RegisterFilter(std::make_unique<XdsHttpRouterFilter>());
RegisterFilter(std::make_unique<XdsHttpFaultFilter>());
RegisterFilter(std::make_unique<XdsHttpRbacFilter>());
}
}
void XdsHttpFilterRegistry::Shutdown() {
delete g_filter_registry;
delete g_filters;
}
} // namespace grpc_core } // namespace grpc_core

@ -19,8 +19,12 @@
#include <grpc/support/port_platform.h> #include <grpc/support/port_platform.h>
#include <algorithm>
#include <map>
#include <memory> #include <memory>
#include <string> #include <string>
#include <utility>
#include <vector>
#include "absl/status/status.h" #include "absl/status/status.h"
#include "absl/status/statusor.h" #include "absl/status/statusor.h"
@ -145,16 +149,32 @@ class XdsHttpRouterFilter : public XdsHttpFilterImpl {
class XdsHttpFilterRegistry { class XdsHttpFilterRegistry {
public: public:
static void RegisterFilter(std::unique_ptr<XdsHttpFilterImpl> filter); explicit XdsHttpFilterRegistry(bool register_builtins = true);
// Not copyable.
XdsHttpFilterRegistry(const XdsHttpFilterRegistry&) = delete;
XdsHttpFilterRegistry& operator=(const XdsHttpFilterRegistry&) = delete;
// Movable.
XdsHttpFilterRegistry(XdsHttpFilterRegistry&& other) noexcept
: owning_list_(std::move(other.owning_list_)),
registry_map_(std::move(other.registry_map_)) {}
XdsHttpFilterRegistry& operator=(XdsHttpFilterRegistry&& other) noexcept {
owning_list_ = std::move(other.owning_list_);
registry_map_ = std::move(other.registry_map_);
return *this;
}
void RegisterFilter(std::unique_ptr<XdsHttpFilterImpl> filter);
static const XdsHttpFilterImpl* GetFilterForType( const XdsHttpFilterImpl* GetFilterForType(
absl::string_view proto_type_name); absl::string_view proto_type_name) const;
static void PopulateSymtab(upb_DefPool* symtab); void PopulateSymtab(upb_DefPool* symtab) const;
// Global init and shutdown. private:
static void Init(bool register_builtins = true); std::vector<std::unique_ptr<XdsHttpFilterImpl>> owning_list_;
static void Shutdown(); std::map<absl::string_view, XdsHttpFilterImpl*> registry_map_;
}; };
} // namespace grpc_core } // namespace grpc_core

@ -354,6 +354,9 @@ XdsListenerResource::HttpConnectionManager HttpConnectionManagerParse(
// http_filters // http_filters
{ {
ValidationErrors::ScopedField field(errors, ".http_filters"); ValidationErrors::ScopedField field(errors, ".http_filters");
const auto& http_filter_registry =
static_cast<const GrpcXdsBootstrap&>(context.client->bootstrap())
.http_filter_registry();
size_t num_filters = 0; size_t num_filters = 0;
const auto* http_filters = const auto* http_filters =
envoy_extensions_filters_network_http_connection_manager_v3_HttpConnectionManager_http_filters( envoy_extensions_filters_network_http_connection_manager_v3_HttpConnectionManager_http_filters(
@ -396,8 +399,7 @@ XdsListenerResource::HttpConnectionManager HttpConnectionManagerParse(
auto extension = ExtractXdsExtension(context, typed_config, errors); auto extension = ExtractXdsExtension(context, typed_config, errors);
const XdsHttpFilterImpl* filter_impl = nullptr; const XdsHttpFilterImpl* filter_impl = nullptr;
if (extension.has_value()) { if (extension.has_value()) {
filter_impl = filter_impl = http_filter_registry.GetFilterForType(extension->type);
XdsHttpFilterRegistry::GetFilterForType(extension->type);
} }
if (filter_impl == nullptr) { if (filter_impl == nullptr) {
if (!is_optional) errors->AddError("unsupported filter type"); if (!is_optional) errors->AddError("unsupported filter type");
@ -431,7 +433,7 @@ XdsListenerResource::HttpConnectionManager HttpConnectionManagerParse(
// out of which only one gets added in the final list. // out of which only one gets added in the final list.
for (const auto& http_filter : http_connection_manager.http_filters) { for (const auto& http_filter : http_connection_manager.http_filters) {
const XdsHttpFilterImpl* filter_impl = const XdsHttpFilterImpl* filter_impl =
XdsHttpFilterRegistry::GetFilterForType( http_filter_registry.GetFilterForType(
http_filter.config.config_proto_type_name); http_filter.config.config_proto_type_name);
if (&http_filter != &http_connection_manager.http_filters.back()) { if (&http_filter != &http_connection_manager.http_filters.back()) {
// Filters before the last filter must not be terminal. // Filters before the last filter must not be terminal.

@ -37,6 +37,8 @@
#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.upbdefs.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.upbdefs.h"
#include "upb/def.h" #include "upb/def.h"
#include "src/core/ext/xds/xds_bootstrap_grpc.h"
#include "src/core/ext/xds/xds_client.h"
#include "src/core/ext/xds/xds_common_types.h" #include "src/core/ext/xds/xds_common_types.h"
#include "src/core/ext/xds/xds_http_filters.h" #include "src/core/ext/xds/xds_http_filters.h"
#include "src/core/ext/xds/xds_resource_type.h" #include "src/core/ext/xds/xds_resource_type.h"
@ -206,11 +208,15 @@ class XdsListenerResourceType
bool AllResourcesRequiredInSotW() const override { return true; } bool AllResourcesRequiredInSotW() const override { return true; }
void InitUpbSymtab(upb_DefPool* symtab) const override { void InitUpbSymtab(XdsClient* xds_client,
upb_DefPool* symtab) const override {
envoy_config_listener_v3_Listener_getmsgdef(symtab); envoy_config_listener_v3_Listener_getmsgdef(symtab);
envoy_extensions_filters_network_http_connection_manager_v3_HttpConnectionManager_getmsgdef( envoy_extensions_filters_network_http_connection_manager_v3_HttpConnectionManager_getmsgdef(
symtab); symtab);
XdsHttpFilterRegistry::PopulateSymtab(symtab); const auto& http_filter_registry =
static_cast<const GrpcXdsBootstrap&>(xds_client->bootstrap())
.http_filter_registry();
http_filter_registry.PopulateSymtab(symtab);
} }
}; };

@ -95,7 +95,8 @@ class XdsResourceType {
// properly in logs. // properly in logs.
// Note: This won't actually work properly until upb adds support for // Note: This won't actually work properly until upb adds support for
// Any fields in textproto printing (internal b/178821188). // Any fields in textproto printing (internal b/178821188).
virtual void InitUpbSymtab(upb_DefPool* symtab) const = 0; virtual void InitUpbSymtab(XdsClient* xds_client,
upb_DefPool* symtab) const = 0;
}; };
} // namespace grpc_core } // namespace grpc_core

@ -333,6 +333,9 @@ ClusterSpecifierPluginParse(
const envoy_config_route_v3_RouteConfiguration* route_config) { const envoy_config_route_v3_RouteConfiguration* route_config) {
XdsRouteConfigResource::ClusterSpecifierPluginMap XdsRouteConfigResource::ClusterSpecifierPluginMap
cluster_specifier_plugin_map; cluster_specifier_plugin_map;
const auto& cluster_specifier_plugin_registry =
static_cast<const GrpcXdsBootstrap&>(context.client->bootstrap())
.cluster_specifier_plugin_registry();
size_t num_cluster_specifier_plugins; size_t num_cluster_specifier_plugins;
const envoy_config_route_v3_ClusterSpecifierPlugin* const* const envoy_config_route_v3_ClusterSpecifierPlugin* const*
cluster_specifier_plugin = cluster_specifier_plugin =
@ -368,7 +371,7 @@ ClusterSpecifierPluginParse(
bool is_optional = envoy_config_route_v3_ClusterSpecifierPlugin_is_optional( bool is_optional = envoy_config_route_v3_ClusterSpecifierPlugin_is_optional(
cluster_specifier_plugin[i]); cluster_specifier_plugin[i]);
const XdsClusterSpecifierPluginImpl* cluster_specifier_plugin_impl = const XdsClusterSpecifierPluginImpl* cluster_specifier_plugin_impl =
XdsClusterSpecifierPluginRegistry::GetPluginForType(extension->type); cluster_specifier_plugin_registry.GetPluginForType(extension->type);
std::string lb_policy_config; std::string lb_policy_config;
if (cluster_specifier_plugin_impl == nullptr) { if (cluster_specifier_plugin_impl == nullptr) {
if (!is_optional) { if (!is_optional) {
@ -631,8 +634,11 @@ ParseTypedPerFilterConfig(
return errors.status("could not determine extension type"); return errors.status("could not determine extension type");
} }
GPR_ASSERT(extension.has_value()); GPR_ASSERT(extension.has_value());
const auto& http_filter_registry =
static_cast<const GrpcXdsBootstrap&>(context.client->bootstrap())
.http_filter_registry();
const XdsHttpFilterImpl* filter_impl = const XdsHttpFilterImpl* filter_impl =
XdsHttpFilterRegistry::GetFilterForType(extension->type); http_filter_registry.GetFilterForType(extension->type);
if (filter_impl == nullptr) { if (filter_impl == nullptr) {
if (is_optional) continue; if (is_optional) continue;
return absl::InvalidArgumentError(absl::StrCat( return absl::InvalidArgumentError(absl::StrCat(

@ -36,6 +36,8 @@
#include "re2/re2.h" #include "re2/re2.h"
#include "upb/def.h" #include "upb/def.h"
#include "src/core/ext/xds/xds_bootstrap_grpc.h"
#include "src/core/ext/xds/xds_client.h"
#include "src/core/ext/xds/xds_cluster_specifier_plugin.h" #include "src/core/ext/xds/xds_cluster_specifier_plugin.h"
#include "src/core/ext/xds/xds_http_filters.h" #include "src/core/ext/xds/xds_http_filters.h"
#include "src/core/ext/xds/xds_resource_type.h" #include "src/core/ext/xds/xds_resource_type.h"
@ -224,9 +226,13 @@ class XdsRouteConfigResourceType
DecodeResult Decode(const XdsResourceType::DecodeContext& context, DecodeResult Decode(const XdsResourceType::DecodeContext& context,
absl::string_view serialized_resource) const override; absl::string_view serialized_resource) const override;
void InitUpbSymtab(upb_DefPool* symtab) const override { void InitUpbSymtab(XdsClient* xds_client,
upb_DefPool* symtab) const override {
envoy_config_route_v3_RouteConfiguration_getmsgdef(symtab); envoy_config_route_v3_RouteConfiguration_getmsgdef(symtab);
XdsClusterSpecifierPluginRegistry::PopulateSymtab(symtab); const auto& cluster_specifier_plugin_registry =
static_cast<const GrpcXdsBootstrap&>(xds_client->bootstrap())
.cluster_specifier_plugin_registry();
cluster_specifier_plugin_registry.PopulateSymtab(symtab);
} }
}; };

@ -220,6 +220,7 @@ const XdsHttpFilterImpl::FilterConfig* FindFilterConfigOverride(
absl::StatusOr<XdsRouting::GeneratePerHttpFilterConfigsResult> absl::StatusOr<XdsRouting::GeneratePerHttpFilterConfigsResult>
XdsRouting::GeneratePerHTTPFilterConfigs( XdsRouting::GeneratePerHTTPFilterConfigs(
const XdsHttpFilterRegistry& http_filter_registry,
const std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>& const std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>&
http_filters, http_filters,
const XdsRouteConfigResource::VirtualHost& vhost, const XdsRouteConfigResource::VirtualHost& vhost,
@ -233,7 +234,7 @@ XdsRouting::GeneratePerHTTPFilterConfigs(
// Find filter. This is guaranteed to succeed, because it's checked // Find filter. This is guaranteed to succeed, because it's checked
// at config validation time in the XdsApi code. // at config validation time in the XdsApi code.
const XdsHttpFilterImpl* filter_impl = const XdsHttpFilterImpl* filter_impl =
XdsHttpFilterRegistry::GetFilterForType( http_filter_registry.GetFilterForType(
http_filter.config.config_proto_type_name); http_filter.config.config_proto_type_name);
GPR_ASSERT(filter_impl != nullptr); GPR_ASSERT(filter_impl != nullptr);
// If there is not actually any C-core filter associated with this // If there is not actually any C-core filter associated with this

@ -31,6 +31,7 @@
#include "absl/strings/string_view.h" #include "absl/strings/string_view.h"
#include "absl/types/optional.h" #include "absl/types/optional.h"
#include "src/core/ext/xds/xds_http_filters.h"
#include "src/core/ext/xds/xds_listener.h" #include "src/core/ext/xds/xds_listener.h"
#include "src/core/ext/xds/xds_route_config.h" #include "src/core/ext/xds/xds_route_config.h"
#include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_args.h"
@ -90,6 +91,7 @@ class XdsRouting {
// Generates a map of per_filter_configs. \a args is consumed. // Generates a map of per_filter_configs. \a args is consumed.
static absl::StatusOr<GeneratePerHttpFilterConfigsResult> static absl::StatusOr<GeneratePerHttpFilterConfigsResult>
GeneratePerHTTPFilterConfigs( GeneratePerHTTPFilterConfigs(
const XdsHttpFilterRegistry& http_filter_registry,
const std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>& const std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>&
http_filters, http_filters,
const XdsRouteConfigResource::VirtualHost& vhost, const XdsRouteConfigResource::VirtualHost& vhost,

@ -315,6 +315,7 @@ class XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
XdsServerConfigSelector : public ServerConfigSelector { XdsServerConfigSelector : public ServerConfigSelector {
public: public:
static absl::StatusOr<RefCountedPtr<XdsServerConfigSelector>> Create( static absl::StatusOr<RefCountedPtr<XdsServerConfigSelector>> Create(
const XdsHttpFilterRegistry& http_filter_registry,
XdsRouteConfigResource rds_update, XdsRouteConfigResource rds_update,
const std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>& const std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>&
http_filters); http_filters);
@ -378,12 +379,18 @@ class XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
: public ServerConfigSelectorProvider { : public ServerConfigSelectorProvider {
public: public:
StaticXdsServerConfigSelectorProvider( StaticXdsServerConfigSelectorProvider(
RefCountedPtr<GrpcXdsClient> xds_client,
absl::StatusOr<XdsRouteConfigResource> static_resource, absl::StatusOr<XdsRouteConfigResource> static_resource,
std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter> std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>
http_filters) http_filters)
: static_resource_(std::move(static_resource)), : xds_client_(std::move(xds_client)),
static_resource_(std::move(static_resource)),
http_filters_(std::move(http_filters)) {} http_filters_(std::move(http_filters)) {}
~StaticXdsServerConfigSelectorProvider() override {
xds_client_.reset(DEBUG_LOCATION, "StaticXdsServerConfigSelectorProvider");
}
absl::StatusOr<RefCountedPtr<ServerConfigSelector>> Watch( absl::StatusOr<RefCountedPtr<ServerConfigSelector>> Watch(
std::unique_ptr<ServerConfigSelectorProvider::ServerConfigSelectorWatcher> std::unique_ptr<ServerConfigSelectorProvider::ServerConfigSelectorWatcher>
watcher) override { watcher) override {
@ -392,8 +399,10 @@ class XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
if (!static_resource_.ok()) { if (!static_resource_.ok()) {
return static_resource_.status(); return static_resource_.status();
} }
return XdsServerConfigSelector::Create(static_resource_.value(), return XdsServerConfigSelector::Create(
http_filters_); static_cast<const GrpcXdsBootstrap&>(xds_client_->bootstrap())
.http_filter_registry(),
static_resource_.value(), http_filters_);
} }
void Orphan() override {} void Orphan() override {}
@ -401,6 +410,7 @@ class XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
void CancelWatch() override { watcher_.reset(); } void CancelWatch() override { watcher_.reset(); }
private: private:
RefCountedPtr<GrpcXdsClient> xds_client_;
absl::StatusOr<XdsRouteConfigResource> static_resource_; absl::StatusOr<XdsRouteConfigResource> static_resource_;
std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter> std::vector<XdsListenerResource::HttpConnectionManager::HttpFilter>
http_filters_; http_filters_;
@ -1051,16 +1061,18 @@ absl::StatusOr<ChannelArgs> XdsServerConfigFetcher::ListenerWatcher::
RefCountedPtr<ServerConfigSelectorProvider> server_config_selector_provider; RefCountedPtr<ServerConfigSelectorProvider> server_config_selector_provider;
RefCountedPtr<XdsChannelStackModifier> channel_stack_modifier; RefCountedPtr<XdsChannelStackModifier> channel_stack_modifier;
RefCountedPtr<XdsCertificateProvider> xds_certificate_provider; RefCountedPtr<XdsCertificateProvider> xds_certificate_provider;
// Add config selector filter.
std::vector<const grpc_channel_filter*> filters;
// Iterate the list of HTTP filters in reverse since in Core, received data // Iterate the list of HTTP filters in reverse since in Core, received data
// flows *up* the stack. // flows *up* the stack.
std::vector<const grpc_channel_filter*> filters;
const auto& http_filter_registry =
static_cast<const GrpcXdsBootstrap&>(xds_client_->bootstrap())
.http_filter_registry();
for (const auto& http_filter : for (const auto& http_filter :
filter_chain->http_connection_manager.http_filters) { filter_chain->http_connection_manager.http_filters) {
// Find filter. This is guaranteed to succeed, because it's checked // Find filter. This is guaranteed to succeed, because it's checked
// at config validation time in the XdsApi code. // at config validation time in the XdsApi code.
const XdsHttpFilterImpl* filter_impl = const XdsHttpFilterImpl* filter_impl =
XdsHttpFilterRegistry::GetFilterForType( http_filter_registry.GetFilterForType(
http_filter.config.config_proto_type_name); http_filter.config.config_proto_type_name);
GPR_ASSERT(filter_impl != nullptr); GPR_ASSERT(filter_impl != nullptr);
// Some filters like the router filter are no-op filters and do not have // Some filters like the router filter are no-op filters and do not have
@ -1069,6 +1081,7 @@ absl::StatusOr<ChannelArgs> XdsServerConfigFetcher::ListenerWatcher::
filters.push_back(filter_impl->channel_filter()); filters.push_back(filter_impl->channel_filter());
} }
} }
// Add config selector filter.
filters.push_back(&kServerConfigSelectorFilter); filters.push_back(&kServerConfigSelectorFilter);
channel_stack_modifier = channel_stack_modifier =
MakeRefCounted<XdsChannelStackModifier>(std::move(filters)); MakeRefCounted<XdsChannelStackModifier>(std::move(filters));
@ -1092,6 +1105,8 @@ absl::StatusOr<ChannelArgs> XdsServerConfigFetcher::ListenerWatcher::
[&](const XdsRouteConfigResource& route_config) { [&](const XdsRouteConfigResource& route_config) {
server_config_selector_provider = server_config_selector_provider =
MakeRefCounted<StaticXdsServerConfigSelectorProvider>( MakeRefCounted<StaticXdsServerConfigSelectorProvider>(
xds_client_->Ref(DEBUG_LOCATION,
"StaticXdsServerConfigSelectorProvider"),
route_config, route_config,
filter_chain->http_connection_manager.http_filters); filter_chain->http_connection_manager.http_filters);
}); });
@ -1122,6 +1137,7 @@ absl::StatusOr<
FilterChainMatchManager::XdsServerConfigSelector>> FilterChainMatchManager::XdsServerConfigSelector>>
XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager:: XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
XdsServerConfigSelector::Create( XdsServerConfigSelector::Create(
const XdsHttpFilterRegistry& http_filter_registry,
XdsRouteConfigResource rds_update, XdsRouteConfigResource rds_update,
const std::vector< const std::vector<
XdsListenerResource::HttpConnectionManager::HttpFilter>& XdsListenerResource::HttpConnectionManager::HttpFilter>&
@ -1139,7 +1155,8 @@ XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
absl::get_if<XdsRouteConfigResource::Route::NonForwardingAction>( absl::get_if<XdsRouteConfigResource::Route::NonForwardingAction>(
&route.action) == nullptr; &route.action) == nullptr;
auto result = XdsRouting::GeneratePerHTTPFilterConfigs( auto result = XdsRouting::GeneratePerHTTPFilterConfigs(
http_filters, vhost, route, nullptr, ChannelArgs()); http_filter_registry, http_filters, vhost, route, nullptr,
ChannelArgs());
if (!result.ok()) return result.status(); if (!result.ok()) return result.status();
std::vector<std::string> fields; std::vector<std::string> fields;
fields.reserve(result->per_filter_configs.size()); fields.reserve(result->per_filter_configs.size());
@ -1265,7 +1282,10 @@ XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
if (!resource.ok()) { if (!resource.ok()) {
return resource.status(); return resource.status();
} }
return XdsServerConfigSelector::Create(resource.value(), http_filters_); return XdsServerConfigSelector::Create(
static_cast<const GrpcXdsBootstrap&>(xds_client_->bootstrap())
.http_filter_registry(),
resource.value(), http_filters_);
} }
void XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager:: void XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
@ -1286,8 +1306,10 @@ void XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::
// DynamicXdsServerConfigSelectorProvider while holding a lock, but if that // DynamicXdsServerConfigSelectorProvider while holding a lock, but if that
// ever changes, we would want to invoke the update outside the critical // ever changes, we would want to invoke the update outside the critical
// region with the use of a WorkSerializer. // region with the use of a WorkSerializer.
watcher_->OnServerConfigSelectorUpdate( watcher_->OnServerConfigSelectorUpdate(XdsServerConfigSelector::Create(
XdsServerConfigSelector::Create(*resource_, http_filters_)); static_cast<const GrpcXdsBootstrap&>(xds_client_->bootstrap())
.http_filter_registry(),
*resource_, http_filters_));
} }
void XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager:: void XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager::

@ -355,7 +355,7 @@ class XdsClientTest : public ::testing::Test {
} }
return result; return result;
} }
void InitUpbSymtab(upb_DefPool* /*symtab*/) const override {} void InitUpbSymtab(XdsClient*, upb_DefPool* /*symtab*/) const override {}
static google::protobuf::Any EncodeAsAny(const ResourceStruct& resource) { static google::protobuf::Any EncodeAsAny(const ResourceStruct& resource) {
google::protobuf::Any any; google::protobuf::Any any;

@ -72,12 +72,6 @@ using ::envoy::extensions::filters::http::router::v3::Router;
class XdsHttpFilterTest : public ::testing::Test { class XdsHttpFilterTest : public ::testing::Test {
protected: protected:
XdsHttpFilterTest() {
// Start with a clean registry for each test.
XdsHttpFilterRegistry::Shutdown();
XdsHttpFilterRegistry::Init();
}
XdsExtension MakeXdsExtension(const grpc::protobuf::Message& message) { XdsExtension MakeXdsExtension(const grpc::protobuf::Message& message) {
google::protobuf::Any any; google::protobuf::Any any;
any.PackFrom(message); any.PackFrom(message);
@ -93,11 +87,12 @@ class XdsHttpFilterTest : public ::testing::Test {
return extension; return extension;
} }
static const XdsHttpFilterImpl* GetFilter(absl::string_view type) { const XdsHttpFilterImpl* GetFilter(absl::string_view type) {
return XdsHttpFilterRegistry::GetFilterForType( return registry_.GetFilterForType(
absl::StripPrefix(type, "type.googleapis.com/")); absl::StripPrefix(type, "type.googleapis.com/"));
} }
XdsHttpFilterRegistry registry_;
ValidationErrors errors_; ValidationErrors errors_;
upb::Arena arena_; upb::Arena arena_;
std::string type_url_storage_; std::string type_url_storage_;
@ -112,15 +107,14 @@ using XdsHttpFilterRegistryTest = XdsHttpFilterTest;
TEST_F(XdsHttpFilterRegistryTest, Basic) { TEST_F(XdsHttpFilterRegistryTest, Basic) {
// Start with an empty registry. // Start with an empty registry.
XdsHttpFilterRegistry::Shutdown(); registry_ = XdsHttpFilterRegistry(/*register_builtins=*/false);
XdsHttpFilterRegistry::Init(/*register_builtins=*/false);
// Returns null when a filter has not yet been registered. // Returns null when a filter has not yet been registered.
XdsExtension extension = MakeXdsExtension(Router()); XdsExtension extension = MakeXdsExtension(Router());
EXPECT_EQ(GetFilter(extension.type), nullptr); EXPECT_EQ(GetFilter(extension.type), nullptr);
// Now register the filter. // Now register the filter.
auto filter = std::make_unique<XdsHttpRouterFilter>(); auto filter = std::make_unique<XdsHttpRouterFilter>();
auto* filter_ptr = filter.get(); auto* filter_ptr = filter.get();
XdsHttpFilterRegistry::RegisterFilter(std::move(filter)); registry_.RegisterFilter(std::move(filter));
// And check that it is now present. // And check that it is now present.
EXPECT_EQ(GetFilter(extension.type), filter_ptr); EXPECT_EQ(GetFilter(extension.type), filter_ptr);
} }
@ -131,9 +125,7 @@ TEST_F(XdsHttpFilterRegistryDeathTest, DuplicateRegistryFails) {
GTEST_FLAG_SET(death_test_style, "threadsafe"); GTEST_FLAG_SET(death_test_style, "threadsafe");
ASSERT_DEATH( ASSERT_DEATH(
// The router filter is already in the registry. // The router filter is already in the registry.
XdsHttpFilterRegistry::RegisterFilter( registry_.RegisterFilter(std::make_unique<XdsHttpRouterFilter>()), "");
std::make_unique<XdsHttpRouterFilter>()),
"");
} }
// //

Loading…
Cancel
Save