From 76b68285e300b16cfe648d9315adf97869e6bb3f Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Wed, 9 Jan 2019 19:16:43 -0800 Subject: [PATCH] config: add support Any as opaque config (#5435) Add support of Any as opaque config for extensions. Deprecates Struct configs. Fixes #4475. Risk Level: Low Testing: CI Docs Changes: Added. Release Notes: Added. Signed-off-by: Lizan Zhou Signed-off-by: Fred Douglas --- DEPRECATED.md | 4 +- api/envoy/api/v2/cds.proto | 3 +- api/envoy/api/v2/core/base.proto | 3 +- api/envoy/api/v2/core/grpc_service.proto | 3 +- api/envoy/api/v2/core/health_check.proto | 3 +- api/envoy/api/v2/listener/listener.proto | 6 +- api/envoy/api/v2/route/route.proto | 15 +-- .../filter/accesslog/v2/accesslog.proto | 3 +- .../v2/http_connection_manager.proto | 3 +- .../thrift_proxy/v2alpha1/thrift_proxy.proto | 3 +- api/envoy/config/metrics/v2/stats.proto | 3 +- .../config/overload/v2alpha/overload.proto | 3 +- api/envoy/config/trace/v2/trace.proto | 3 +- docs/root/intro/version_history.rst | 1 + source/common/config/utility.cc | 31 +++++ source/common/config/utility.h | 57 ++------- source/common/router/config_impl.cc | 54 +++++--- source/common/router/config_impl.h | 3 +- source/common/upstream/upstream_impl.cc | 64 +++++++--- .../common/access_log/access_log_impl_test.cc | 23 ++++ test/common/router/config_impl_test.cc | 120 +++++++++++++++++- test/common/upstream/upstream_impl_test.cc | 93 ++++++++++++++ test/config/integration/server.yaml | 3 +- test/config/utility.cc | 2 +- test/extensions/tracers/zipkin/config_test.cc | 23 ++++ 25 files changed, 406 insertions(+), 123 deletions(-) diff --git a/DEPRECATED.md b/DEPRECATED.md index 1f209988900d..d7b6997c034a 100644 --- a/DEPRECATED.md +++ b/DEPRECATED.md @@ -10,8 +10,10 @@ A logged warning is expected for each deprecated item that is in deprecation win * Use of `enabled` in `CorsPolicy`, found in [route.proto](https://github.com/envoyproxy/envoy/blob/master/api/envoy/api/v2/route/route.proto). Set the `filter_enabled` field instead. +* Use of google.protobuf.Struct for extension opaque configs is deprecated. Use google.protobuf.Any instead or pack +google.protobuf.Struct in google.protobuf.Any. -## Version 1.9.0 +## Version 1.9.0 (Dec 20, 2018) * Order of execution of the network write filter chain has been reversed. Prior to this release cycle it was incorrect, see [#4599](https://github.com/envoyproxy/envoy/issues/4599). In the 1.9.0 release cycle we introduced `bugfix_reverse_write_filter_order` in [lds.proto] (https://github.com/envoyproxy/envoy/blob/master/api/envoy/api/v2/lds.proto) to temporarily support both old and new behaviors. Note this boolean field is deprecated. * Order of execution of the HTTP encoder filter chain has been reversed. Prior to this release cycle it was incorrect, see [#4599](https://github.com/envoyproxy/envoy/issues/4599). In the 1.9.0 release cycle we introduced `bugfix_reverse_encode_order` in [http_connection_manager.proto] (https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto) to temporarily support both old and new behaviors. Note this boolean field is deprecated. diff --git a/api/envoy/api/v2/cds.proto b/api/envoy/api/v2/cds.proto index e8f86e9bfcc2..d4daac7bd806 100644 --- a/api/envoy/api/v2/cds.proto +++ b/api/envoy/api/v2/cds.proto @@ -233,9 +233,8 @@ message Cluster { // for upstream connections. The key should match the extension filter name, such as // "envoy.filters.network.thrift_proxy". See the extension's documentation for details on // specific options. - map extension_protocol_options = 35; + map extension_protocol_options = 35 [deprecated = true]; - // [#not-implemented-hide:] // The extension_protocol_options field is used to provide extension-specific protocol options // for upstream connections. The key should match the extension filter name, such as // "envoy.filters.network.thrift_proxy". See the extension's documentation for details on diff --git a/api/envoy/api/v2/core/base.proto b/api/envoy/api/v2/core/base.proto index 82482099574f..e764df9c487a 100644 --- a/api/envoy/api/v2/core/base.proto +++ b/api/envoy/api/v2/core/base.proto @@ -190,9 +190,8 @@ message TransportSocket { // Implementation specific configuration which depends on the implementation being instantiated. // See the supported transport socket implementations for further documentation. oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 3; } } diff --git a/api/envoy/api/v2/core/grpc_service.proto b/api/envoy/api/v2/core/grpc_service.proto index 7a009d813ea5..923a9313a3e1 100644 --- a/api/envoy/api/v2/core/grpc_service.proto +++ b/api/envoy/api/v2/core/grpc_service.proto @@ -82,9 +82,8 @@ message GrpcService { message MetadataCredentialsFromPlugin { string name = 1; oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 3; } } diff --git a/api/envoy/api/v2/core/health_check.proto b/api/envoy/api/v2/core/health_check.proto index ea2e245a7772..24082c1f0253 100644 --- a/api/envoy/api/v2/core/health_check.proto +++ b/api/envoy/api/v2/core/health_check.proto @@ -163,9 +163,8 @@ message HealthCheck { // A custom health checker specific configuration which depends on the custom health checker // being instantiated. See :api:`envoy/config/health_checker` for reference. oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 3; } } diff --git a/api/envoy/api/v2/listener/listener.proto b/api/envoy/api/v2/listener/listener.proto index 0d617ceacf5f..6824de9631c9 100644 --- a/api/envoy/api/v2/listener/listener.proto +++ b/api/envoy/api/v2/listener/listener.proto @@ -39,9 +39,8 @@ message Filter { // Filter specific configuration which depends on the filter being // instantiated. See the supported filters for further documentation. oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 4; } @@ -209,9 +208,8 @@ message ListenerFilter { // Filter specific configuration which depends on the filter being instantiated. // See the supported filters for further documentation. oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 3; } } diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index 27554e31db27..9a8ec23ce8eb 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -113,9 +113,8 @@ message VirtualHost { // *envoy.buffer* for the HTTP buffer filter. Use of this field is filter // specific; see the :ref:`HTTP filter documentation ` // for if and how it is utilized. - map per_filter_config = 12; + map per_filter_config = 12 [deprecated = true]; - // [#not-implemented-hide:] // The per_filter_config field can be used to provide virtual host-specific // configurations for filters. The key should match the filter name, such as // *envoy.buffer* for the HTTP buffer filter. Use of this field is filter @@ -181,9 +180,8 @@ message Route { // *envoy.buffer* for the HTTP buffer filter. Use of this field is filter // specific; see the :ref:`HTTP filter documentation ` for // if and how it is utilized. - map per_filter_config = 8; + map per_filter_config = 8 [deprecated = true]; - // [#not-implemented-hide:] // The per_filter_config field can be used to provide route-specific // configurations for filters. The key should match the filter name, such as // *envoy.buffer* for the HTTP buffer filter. Use of this field is filter @@ -279,9 +277,8 @@ message WeightedCluster { // *envoy.buffer* for the HTTP buffer filter. Use of this field is filter // specific; see the :ref:`HTTP filter documentation ` // for if and how it is utilized. - map per_filter_config = 8; + map per_filter_config = 8 [deprecated = true]; - // [#not-implemented-hide:] // The per_filter_config field can be used to provide weighted cluster-specific // configurations for filters. The key should match the filter name, such as // *envoy.buffer* for the HTTP buffer filter. Use of this field is filter @@ -805,9 +802,8 @@ message RetryPolicy { message RetryPriority { string name = 1 [(validate.rules).string.min_bytes = 1]; oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 3; } } @@ -820,9 +816,8 @@ message RetryPolicy { message RetryHostPredicate { string name = 1 [(validate.rules).string.min_bytes = 1]; oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 3; } } diff --git a/api/envoy/config/filter/accesslog/v2/accesslog.proto b/api/envoy/config/filter/accesslog/v2/accesslog.proto index 76f5994af344..0ec539d1d66c 100644 --- a/api/envoy/config/filter/accesslog/v2/accesslog.proto +++ b/api/envoy/config/filter/accesslog/v2/accesslog.proto @@ -35,9 +35,8 @@ message AccessLog { // #. "envoy.http_grpc_access_log": :ref:`HttpGrpcAccessLogConfig // ` oneof config_type { - google.protobuf.Struct config = 3; + google.protobuf.Struct config = 3 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 4; } } diff --git a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto index 440188ef9f33..b6da80c64732 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto @@ -410,9 +410,8 @@ message HttpFilter { // Filter specific configuration which depends on the filter being instantiated. See the supported // filters for further documentation. oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 4; } diff --git a/api/envoy/config/filter/network/thrift_proxy/v2alpha1/thrift_proxy.proto b/api/envoy/config/filter/network/thrift_proxy/v2alpha1/thrift_proxy.proto index 7d17a6aab9ce..4887bc11cb51 100644 --- a/api/envoy/config/filter/network/thrift_proxy/v2alpha1/thrift_proxy.proto +++ b/api/envoy/config/filter/network/thrift_proxy/v2alpha1/thrift_proxy.proto @@ -95,9 +95,8 @@ message ThriftFilter { // Filter specific configuration which depends on the filter being instantiated. See the supported // filters for further documentation. oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 3; } } diff --git a/api/envoy/config/metrics/v2/stats.proto b/api/envoy/config/metrics/v2/stats.proto index 1cd4b146f8d2..3bd205401c98 100644 --- a/api/envoy/config/metrics/v2/stats.proto +++ b/api/envoy/config/metrics/v2/stats.proto @@ -33,9 +33,8 @@ message StatsSink { // Stats sink specific configuration which depends on the sink being instantiated. See // :ref:`StatsdSink ` for an example. oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 3; } } diff --git a/api/envoy/config/overload/v2alpha/overload.proto b/api/envoy/config/overload/v2alpha/overload.proto index cbb34e17b7f4..a22b1679f4eb 100644 --- a/api/envoy/config/overload/v2alpha/overload.proto +++ b/api/envoy/config/overload/v2alpha/overload.proto @@ -30,9 +30,8 @@ message ResourceMonitor { // Configuration for the resource monitor being instantiated. oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 3; } } diff --git a/api/envoy/config/trace/v2/trace.proto b/api/envoy/config/trace/v2/trace.proto index 058bc99bfcff..607ce7e6a184 100644 --- a/api/envoy/config/trace/v2/trace.proto +++ b/api/envoy/config/trace/v2/trace.proto @@ -41,9 +41,8 @@ message Tracing { // - :ref:`DynamicOtConfig ` // - :ref:`DatadogConfig ` oneof config_type { - google.protobuf.Struct config = 2; + google.protobuf.Struct config = 2 [deprecated = true]; - // [#not-implemented-hide:] google.protobuf.Any typed_config = 3; } } diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 21d36b1e9e5c..9a9ca7a53eb8 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -3,6 +3,7 @@ Version history 1.10.0 (pending) ================ +* config: added support of using google.protobuf.Any in opaque configs for extensions. * access log: added a new flag for upstream retry count exceeded. * admin: the admin server can now be accessed via HTTP/2 (prior knowledge). * buffer: fix vulnerabilities when allocation fails diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc index d47f9c01baaf..69fa5a733ac5 100644 --- a/source/common/config/utility.cc +++ b/source/common/config/utility.cc @@ -278,5 +278,36 @@ envoy::api::v2::ClusterLoadAssignment Utility::translateClusterHosts( return load_assignment; } +void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config, + const ProtobufWkt::Struct& config, + Protobuf::Message& out_proto) { + static const std::string& struct_type = + ProtobufWkt::Struct::default_instance().GetDescriptor()->full_name(); + + if (!typed_config.value().empty()) { + + // Unpack methods will only use the fully qualified type name after the last '/'. + // https://github.com/protocolbuffers/protobuf/blob/3.6.x/src/google/protobuf/any.proto#L87 + absl::string_view type = typed_config.type_url(); + size_t pos = type.find_last_of('/'); + if (pos != absl::string_view::npos) { + type = type.substr(pos + 1); + } + + // out_proto is expecting Struct, unpack directly + if (type != struct_type || out_proto.GetDescriptor()->full_name() == struct_type) { + typed_config.UnpackTo(&out_proto); + } else { + ProtobufWkt::Struct struct_config; + typed_config.UnpackTo(&struct_config); + MessageUtil::jsonConvert(struct_config, out_proto); + } + } + + if (!config.fields().empty()) { + MessageUtil::jsonConvert(config, out_proto); + } +} + } // namespace Config } // namespace Envoy diff --git a/source/common/config/utility.h b/source/common/config/utility.h index cd142bdbb6f7..20653b3580d1 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -280,55 +280,11 @@ class Utility { // Fail in an obvious way if a plugin does not return a proto. RELEASE_ASSERT(config != nullptr, ""); - if (enclosing_message.has_config()) { - MessageUtil::jsonConvert(enclosing_message.config(), *config); - } + translateOpaqueConfig(enclosing_message.typed_config(), enclosing_message.config(), *config); return config; } - /** - * Translate a nested config into a route-specific proto message provided by - * the implementation factory. - * @param source Protobuf::Message containing the opaque config for the given factory's - * route-local configuration. - * @param factory Server::Configuration::NamedHttpFilterConfigFactory implementation - * @return ProtobufTypes::MessagePtr the translated config - */ - static ProtobufTypes::MessagePtr - translateToFactoryRouteConfig(const Protobuf::Message& source, - Server::Configuration::NamedHttpFilterConfigFactory& factory) { - ProtobufTypes::MessagePtr config = factory.createEmptyRouteConfigProto(); - - // Fail in an obvious way if a plugin does not return a proto. - RELEASE_ASSERT(config != nullptr, ""); - - MessageUtil::jsonConvert(source, *config); - return config; - } - - /** - * Translate a nested config into a protocol-specific options proto message provided by the - * implementation factory. - * @param source Protobuf::Message containing the opaque config for the given factory's - * protocol specific configuration. - * @param factory Server::Configuration::NamedNetworkFilterConfigFactory implementation - * @return ProtobufTypes::MessagePtr the translated config - * @throws EnvoyException if the factory does not support protocol options - */ - static ProtobufTypes::MessagePtr - translateToFactoryProtocolOptionsConfig(const Protobuf::Message& source, const std::string& name, - Server::Configuration::ProtocolOptionsFactory& factory) { - ProtobufTypes::MessagePtr config = factory.createEmptyProtocolOptionsProto(); - - if (config == nullptr) { - throw EnvoyException(fmt::format("filter {} does not support protocol options", name)); - } - - MessageUtil::jsonConvert(source, *config); - return config; - } - /** * Create TagProducer instance. Check all tag names for conflicts to avoid * unexpected tag name overwriting. @@ -373,6 +329,17 @@ class Utility { */ static envoy::api::v2::ClusterLoadAssignment translateClusterHosts(const Protobuf::RepeatedPtrField& hosts); + + /** + * Translate opaque config from google.protobuf.Any or google.protobuf.Struct to defined proto + * message. + * @param typed_config opaque config packed in google.protobuf.Any + * @param config the deprecated google.protobuf.Struct config, empty struct if doesn't exist. + * @param out_proto the proto message instantiated by extensions + */ + static void translateOpaqueConfig(const ProtobufWkt::Any& typed_config, + const ProtobufWkt::Struct& config, + Protobuf::Message& out_proto); }; } // namespace Config diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index ad249d0f62f0..ddd796211552 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -321,7 +321,8 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, decorator_(parseDecorator(route)), direct_response_code_(ConfigUtility::parseDirectResponseCode(route)), direct_response_body_(ConfigUtility::parseDirectResponseBody(route)), - per_filter_configs_(route.per_filter_config(), factory_context), + per_filter_configs_(route.typed_per_filter_config(), route.per_filter_config(), + factory_context), time_system_(factory_context.dispatcher().timeSystem()) { if (route.route().has_metadata_match()) { const auto filter_it = route.route().metadata_match().filter_metadata().find( @@ -697,7 +698,8 @@ RouteEntryImplBase::WeightedClusterEntry::WeightedClusterEntry( cluster.request_headers_to_remove())), response_headers_parser_(HeaderParser::configure(cluster.response_headers_to_add(), cluster.response_headers_to_remove())), - per_filter_configs_(cluster.per_filter_config(), factory_context) { + per_filter_configs_(cluster.typed_per_filter_config(), cluster.per_filter_config(), + factory_context) { if (cluster.has_metadata_match()) { const auto filter_it = cluster.metadata_match().filter_metadata().find( Envoy::Config::MetadataFilters::get().ENVOY_LB); @@ -819,7 +821,8 @@ VirtualHostImpl::VirtualHostImpl(const envoy::api::v2::route::VirtualHost& virtu virtual_host.request_headers_to_remove())), response_headers_parser_(HeaderParser::configure(virtual_host.response_headers_to_add(), virtual_host.response_headers_to_remove())), - per_filter_configs_(virtual_host.per_filter_config(), factory_context), + per_filter_configs_(virtual_host.typed_per_filter_config(), virtual_host.per_filter_config(), + factory_context), include_attempt_count_(virtual_host.include_request_attempt_count()) { switch (virtual_host.require_tls()) { case envoy::api::v2::route::VirtualHost::NONE: @@ -1035,21 +1038,42 @@ ConfigImpl::ConfigImpl(const envoy::api::v2::RouteConfiguration& config, config.response_headers_to_remove()); } +namespace { + +RouteSpecificFilterConfigConstSharedPtr +createRouteSpecificFilterConfig(const std::string& name, const ProtobufWkt::Any& typed_config, + const ProtobufWkt::Struct& config, + Server::Configuration::FactoryContext& factory_context) { + auto& factory = Envoy::Config::Utility::getAndCheckFactory< + Server::Configuration::NamedHttpFilterConfigFactory>(name); + ProtobufTypes::MessagePtr proto_config = factory.createEmptyRouteConfigProto(); + Envoy::Config::Utility::translateOpaqueConfig(typed_config, config, *proto_config); + return factory.createRouteSpecificFilterConfig(*proto_config, factory_context); +} + +} // namespace + PerFilterConfigs::PerFilterConfigs( + const Protobuf::Map& typed_configs, const Protobuf::Map& configs, Server::Configuration::FactoryContext& factory_context) { - for (const auto& cfg : configs) { - const std::string& name = cfg.first; - const ProtobufWkt::Struct& struct_config = cfg.second; - - auto& factory = Envoy::Config::Utility::getAndCheckFactory< - Server::Configuration::NamedHttpFilterConfigFactory>(name); - - auto object = factory.createRouteSpecificFilterConfig( - *Envoy::Config::Utility::translateToFactoryRouteConfig(struct_config, factory), - factory_context); - if (object) { - configs_[name] = object; + if (!typed_configs.empty() && !configs.empty()) { + throw EnvoyException("Only one of typed_configs or configs can be specified"); + } + + for (const auto& it : typed_configs) { + auto object = createRouteSpecificFilterConfig( + it.first, it.second, ProtobufWkt::Struct::default_instance(), factory_context); + if (object != nullptr) { + configs_[it.first] = std::move(object); + } + } + + for (const auto& it : configs) { + auto object = createRouteSpecificFilterConfig(it.first, ProtobufWkt::Any::default_instance(), + it.second, factory_context); + if (object != nullptr) { + configs_[it.first] = std::move(object); } } } diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index d2004cb21220..a5b22eea9a43 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -50,7 +50,8 @@ class Matchable { class PerFilterConfigs { public: - PerFilterConfigs(const Protobuf::Map& configs, + PerFilterConfigs(const Protobuf::Map& typed_configs, + const Protobuf::Map& configs, Server::Configuration::FactoryContext& factory_context); const RouteSpecificFilterConfig* get(const std::string& name) const; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index fad99d40cd86..300246aa246e 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -113,31 +113,57 @@ parseClusterSocketOptions(const envoy::api::v2::Cluster& config, return cluster_options; } +ProtocolOptionsConfigConstSharedPtr +createProtocolOptionsConfig(const std::string& name, const ProtobufWkt::Any& typed_config, + const ProtobufWkt::Struct& config) { + Server::Configuration::ProtocolOptionsFactory* factory = + Registry::FactoryRegistry::getFactory( + name); + if (factory == nullptr) { + factory = + Registry::FactoryRegistry::getFactory( + name); + } + + if (factory == nullptr) { + throw EnvoyException(fmt::format( + "Didn't find a registered network or http filter implementation for name: '{}'", name)); + } + + ProtobufTypes::MessagePtr proto_config = factory->createEmptyProtocolOptionsProto(); + + if (proto_config == nullptr) { + throw EnvoyException(fmt::format("filter {} does not support protocol options", name)); + } + + Envoy::Config::Utility::translateOpaqueConfig(typed_config, config, *proto_config); + + return factory->createProtocolOptionsConfig(*proto_config); +} + std::map parseExtensionProtocolOptions(const envoy::api::v2::Cluster& config) { + if (!config.typed_extension_protocol_options().empty() && + !config.extension_protocol_options().empty()) { + throw EnvoyException("Only one of typed_extension_protocol_options or " + "extension_protocol_options can be specified"); + } + std::map options; - for (const auto& iter : config.extension_protocol_options()) { - const std::string& name = iter.first; - const ProtobufWkt::Struct& config_struct = iter.second; - Server::Configuration::ProtocolOptionsFactory* factory = nullptr; - - factory = Registry::FactoryRegistry< - Server::Configuration::NamedNetworkFilterConfigFactory>::getFactory(name); - if (factory == nullptr) { - factory = Registry::FactoryRegistry< - Server::Configuration::NamedHttpFilterConfigFactory>::getFactory(name); - } - if (factory == nullptr) { - throw EnvoyException(fmt::format( - "Didn't find a registered network or http filter implementation for name: '{}'", name)); + for (const auto& it : config.typed_extension_protocol_options()) { + auto object = + createProtocolOptionsConfig(it.first, it.second, ProtobufWkt::Struct::default_instance()); + if (object != nullptr) { + options[it.first] = std::move(object); } + } - auto object = factory->createProtocolOptionsConfig( - *Envoy::Config::Utility::translateToFactoryProtocolOptionsConfig(config_struct, name, - *factory)); - if (object) { - options[name] = object; + for (const auto& it : config.extension_protocol_options()) { + auto object = + createProtocolOptionsConfig(it.first, ProtobufWkt::Any::default_instance(), it.second); + if (object != nullptr) { + options[it.first] = std::move(object); } } diff --git a/test/common/access_log/access_log_impl_test.cc b/test/common/access_log/access_log_impl_test.cc index d751d5e3ee2a..f41b113d5c30 100644 --- a/test/common/access_log/access_log_impl_test.cc +++ b/test/common/access_log/access_log_impl_test.cc @@ -895,6 +895,29 @@ name: envoy.file_access_log "response_flag_filter {\n flags: \"UnsupportedFlag\"\n}\n"); } +TEST_F(AccessLogImplTest, ValidateTypedConfig) { + const std::string yaml = R"EOF( +name: envoy.file_access_log +filter: + response_flag_filter: + flags: + - UnsupportedFlag +typed_config: + "@type": type.googleapis.com/envoy.config.accesslog.v2.FileAccessLog + path: /dev/null + )EOF"; + + EXPECT_THROW_WITH_MESSAGE( + AccessLogFactory::fromProto(parseAccessLogFromV2Yaml(yaml), context_), + ProtoValidationException, + "Proto constraint validation failed (AccessLogFilterValidationError.ResponseFlagFilter: " + "[\"embedded message failed validation\"] | caused by " + "ResponseFlagFilterValidationError.Flags[i]: [\"value must be in list \" [\"LH\" \"UH\" " + "\"UT\" \"LR\" \"UR\" \"UF\" \"UC\" \"UO\" \"NR\" \"DI\" \"FI\" \"RL\" \"UAEX\" \"RLSE\" " + "\"DC\" \"URX\"]]): " + "response_flag_filter {\n flags: \"UnsupportedFlag\"\n}\n"); +} + } // namespace } // namespace AccessLog } // namespace Envoy diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index f8c47d9c107b..4324bed87815 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -5401,8 +5401,51 @@ class PerFilterConfigsTest : public testing::Test { NiceMock factory_context_; }; +TEST_F(PerFilterConfigsTest, TypedConfigFilterError) { + { + const std::string yaml = R"EOF( +name: foo +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + per_filter_config: { unknown.filter: {} } + typed_per_filter_config: + unknown.filter: + "@type": type.googleapis.com/google.protobuf.Timestamp +)EOF"; + + EXPECT_THROW_WITH_MESSAGE( + TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true), + EnvoyException, "Only one of typed_configs or configs can be specified"); + } + + { + const std::string yaml = R"EOF( +name: foo +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + per_filter_config: { unknown.filter: {} } + typed_per_filter_config: + unknown.filter: + "@type": type.googleapis.com/google.protobuf.Timestamp +)EOF"; + + EXPECT_THROW_WITH_MESSAGE( + TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true), + EnvoyException, "Only one of typed_configs or configs can be specified"); + } +} + TEST_F(PerFilterConfigsTest, UnknownFilter) { - const std::string yaml = R"EOF( + { + const std::string yaml = R"EOF( name: foo virtual_hosts: - name: bar @@ -5413,15 +5456,36 @@ name: foo per_filter_config: { unknown.filter: {} } )EOF"; - EXPECT_THROW_WITH_MESSAGE( - TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true), - EnvoyException, "Didn't find a registered implementation for name: 'unknown.filter'"); + EXPECT_THROW_WITH_MESSAGE( + TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true), + EnvoyException, "Didn't find a registered implementation for name: 'unknown.filter'"); + } + + { + const std::string yaml = R"EOF( +name: foo +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + typed_per_filter_config: + unknown.filter: + "@type": type.googleapis.com/google.protobuf.Timestamp +)EOF"; + + EXPECT_THROW_WITH_MESSAGE( + TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true), + EnvoyException, "Didn't find a registered implementation for name: 'unknown.filter'"); + } } // Test that a trivially specified NamedHttpFilterConfigFactory ignores per_filter_config without // error. TEST_F(PerFilterConfigsTest, DefaultFilterImplementation) { - const std::string yaml = R"EOF( + { + const std::string yaml = R"EOF( name: foo virtual_hosts: - name: bar @@ -5432,7 +5496,27 @@ name: foo per_filter_config: { test.default.filter: { seconds: 123} } )EOF"; - checkNoPerFilterConfig(yaml); + checkNoPerFilterConfig(yaml); + } + + { + const std::string yaml = R"EOF( +name: foo +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + typed_per_filter_config: + test.default.filter: + "@type": type.googleapis.com/google.protobuf.Timestamp + value: + seconds: 123 +)EOF"; + + checkNoPerFilterConfig(yaml); + } } TEST_F(PerFilterConfigsTest, RouteLocalConfig) { @@ -5451,6 +5535,30 @@ name: foo checkEach(yaml, 123, 123, 456); } +TEST_F(PerFilterConfigsTest, RouteLocalTypedConfig) { + const std::string yaml = R"EOF( +name: foo +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } + typed_per_filter_config: + test.filter: + "@type": type.googleapis.com/google.protobuf.Timestamp + value: + seconds: 123 + typed_per_filter_config: + test.filter: + "@type": type.googleapis.com/google.protobuf.Struct + value: + seconds: 456 +)EOF"; + + checkEach(yaml, 123, 123, 456); +} + TEST_F(PerFilterConfigsTest, WeightedClusterConfig) { const std::string yaml = R"EOF( name: foo diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 0469615e3c17..131b8fed1505 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -1699,6 +1699,42 @@ TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForUnknownFilter) { "Didn't find a registered network or http filter implementation for name: 'no_such_filter'"); } +TEST_F(ClusterInfoImplTest, TypedExtensionProtocolOptionsForUnknownFilter) { + const std::string yaml = R"EOF( + name: name + connect_timeout: 0.25s + type: STRICT_DNS + lb_policy: ROUND_ROBIN + hosts: [{ socket_address: { address: foo.bar.com, port_value: 443 }}] + typed_extension_protocol_options: + no_such_filter: + "@type": type.googleapis.com/google.protobuf.Struct + )EOF"; + + EXPECT_THROW_WITH_MESSAGE( + makeCluster(yaml), EnvoyException, + "Didn't find a registered network or http filter implementation for name: 'no_such_filter'"); +} + +TEST_F(ClusterInfoImplTest, OneofExtensionProtocolOptionsForUnknownFilter) { + const std::string yaml = R"EOF( + name: name + connect_timeout: 0.25s + type: STRICT_DNS + lb_policy: ROUND_ROBIN + hosts: [{ socket_address: { address: foo.bar.com, port_value: 443 }}] + extension_protocol_options: + no_such_filter: { option: value } + typed_extension_protocol_options: + no_such_filter: + "@type": type.googleapis.com/google.protobuf.Struct + )EOF"; + + EXPECT_THROW_WITH_MESSAGE(makeCluster(yaml), EnvoyException, + "Only one of typed_extension_protocol_options or " + "extension_protocol_options can be specified"); +} + class TestFilterConfigFactoryBase { public: TestFilterConfigFactoryBase( @@ -1814,6 +1850,37 @@ TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForFilterWithoutOptions) { } } +TEST_F(ClusterInfoImplTest, TypedExtensionProtocolOptionsForFilterWithoutOptions) { + TestFilterConfigFactoryBase factoryBase( + []() -> ProtobufTypes::MessagePtr { return nullptr; }, + [](const Protobuf::Message&) -> Upstream::ProtocolOptionsConfigConstSharedPtr { + return nullptr; + }); + const std::string yaml = R"EOF( + name: name + connect_timeout: 0.25s + type: STRICT_DNS + lb_policy: ROUND_ROBIN + hosts: [{ socket_address: { address: foo.bar.com, port_value: 443 }}] + typed_extension_protocol_options: + envoy.test.filter: { "@type": type.googleapis.com/google.protobuf.Struct } + )EOF"; + + { + TestNetworkFilterConfigFactory factory(factoryBase); + Registry::InjectFactory registry( + factory); + EXPECT_THROW_WITH_MESSAGE(makeCluster(yaml), EnvoyException, + "filter envoy.test.filter does not support protocol options"); + } + { + TestHttpFilterConfigFactory factory(factoryBase); + Registry::InjectFactory registry(factory); + EXPECT_THROW_WITH_MESSAGE(makeCluster(yaml), EnvoyException, + "filter envoy.test.filter does not support protocol options"); + } +} + // Cluster retrieval of typed extension protocol options. TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForFilterWithOptions) { auto protocol_options = std::make_shared(); @@ -1837,6 +1904,19 @@ TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForFilterWithOptions) { envoy.test.filter: { option: "value" } )EOF"; + const std::string typed_yaml = R"EOF( + name: name + connect_timeout: 0.25s + type: STRICT_DNS + lb_policy: ROUND_ROBIN + hosts: [{ socket_address: { address: foo.bar.com, port_value: 443 }}] + typed_extension_protocol_options: + envoy.test.filter: + "@type": type.googleapis.com/google.protobuf.Struct + value: + option: "value" + )EOF"; + // This vector is used to gather clusters with extension_protocol_options from the different // types of extension factories (network, http). std::vector> clusters; @@ -1854,6 +1934,19 @@ TEST_F(ClusterInfoImplTest, ExtensionProtocolOptionsForFilterWithOptions) { Registry::InjectFactory registry(factory); clusters.push_back(makeCluster(yaml)); } + { + // Get the cluster with extension_protocol_options for a network filter factory. + TestNetworkFilterConfigFactory factory(factoryBase); + Registry::InjectFactory registry( + factory); + clusters.push_back(makeCluster(typed_yaml)); + } + { + // Get the cluster with extension_protocol_options for an http filter factory. + TestHttpFilterConfigFactory factory(factoryBase); + Registry::InjectFactory registry(factory); + clusters.push_back(makeCluster(typed_yaml)); + } // Make sure that the clusters created from both factories are as expected. for (auto&& cluster : clusters) { diff --git a/test/config/integration/server.yaml b/test/config/integration/server.yaml index 61255f3660d4..11c5bf992d08 100644 --- a/test/config/integration/server.yaml +++ b/test/config/integration/server.yaml @@ -411,7 +411,8 @@ stats_sinks: address: {{ ip_loopback_address }} port_value: 8125 - name: envoy.statsd - config: + typed_config: + "@type": type.googleapis.com/envoy.config.metrics.v2.StatsdSink tcp_cluster_name: statsd watchdog: {} runtime: diff --git a/test/config/utility.cc b/test/config/utility.cc index 7db26be9e62f..b488015469e6 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -254,7 +254,7 @@ void ConfigHelper::setCaptureTransportSocket( file_sink->set_path_prefix(capture_path + "_" + absl::StrReplaceAll(test_id, {{"/", "_"}})); file_sink->set_format(envoy::config::transport_socket::capture::v2alpha::FileSink::PROTO_TEXT); capture_config.mutable_transport_socket()->MergeFrom(inner_transport_socket); - MessageUtil::jsonConvert(capture_config, *transport_socket.mutable_config()); + transport_socket.mutable_typed_config()->PackFrom(capture_config); } void ConfigHelper::setSourceAddress(const std::string& address_string) { diff --git a/test/extensions/tracers/zipkin/config_test.cc b/test/extensions/tracers/zipkin/config_test.cc index f272c4b84626..017c7344f6b9 100644 --- a/test/extensions/tracers/zipkin/config_test.cc +++ b/test/extensions/tracers/zipkin/config_test.cc @@ -34,6 +34,29 @@ TEST(ZipkinTracerConfigTest, ZipkinHttpTracer) { EXPECT_NE(nullptr, zipkin_tracer); } +TEST(ZipkinTracerConfigTest, ZipkinHttpTracerWithTypedConfig) { + NiceMock server; + EXPECT_CALL(server.cluster_manager_, get("fake_cluster")) + .WillRepeatedly(Return(&server.cluster_manager_.thread_local_cluster_)); + + const std::string yaml_string = R"EOF( + http: + name: envoy.zipkin + typed_config: + "@type": type.googleapis.com/envoy.config.trace.v2.ZipkinConfig + collector_cluster: fake_cluster + collector_endpoint: /api/v1/spans + )EOF"; + + envoy::config::trace::v2::Tracing configuration; + MessageUtil::loadFromYaml(yaml_string, configuration); + + ZipkinTracerFactory factory; + auto message = Config::Utility::translateToFactoryConfig(configuration.http(), factory); + Tracing::HttpTracerPtr zipkin_tracer = factory.createHttpTracer(*message, server); + EXPECT_NE(nullptr, zipkin_tracer); +} + TEST(ZipkinTracerConfigTest, DoubleRegistrationTest) { EXPECT_THROW_WITH_MESSAGE( (Registry::RegisterFactory()),