From 83604ea3f7268d70a8ec9c2118c6adfd54af9a59 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Thu, 17 Aug 2017 18:01:44 -0400 Subject: [PATCH 1/7] config: Struct opaque filter proto config support, LDS/RDS integration test. This PR provides an integration test demoing LDS/RDS/CDS/EDS. Along the way, needed to solve the problem of plumbing in the new HTTP connection manager filter proto config, rather than the legacy v1 JSON. The solution adopted in this PR is to use a "deprecatedV1" field in the opaque config to determine if we're doing legacy config or v2 proto. --- include/envoy/server/BUILD | 2 + include/envoy/server/filter_config.h | 42 ++++++++++++++ source/common/config/filter_json.cc | 6 +- source/common/config/lds_json.cc | 6 +- source/common/protobuf/BUILD | 2 + source/common/protobuf/protobuf.h | 2 + source/common/protobuf/utility.cc | 20 ++++++- source/common/protobuf/utility.h | 12 ++++ .../config/network/http_connection_manager.cc | 56 +++++++++++++------ .../config/network/http_connection_manager.h | 6 ++ source/server/configuration_impl.cc | 6 +- source/server/listener_manager_impl.cc | 30 +++++----- test/config/integration/BUILD | 2 + .../integration/server_xds.bootstrap.json | 3 + test/config/integration/server_xds.cds.json | 28 +++++----- test/config/integration/server_xds.json | 35 +----------- test/config/integration/server_xds.lds.json | 28 ++++++++++ test/config/integration/server_xds.rds.json | 15 +++++ test/integration/integration.cc | 13 ++++- test/integration/integration.h | 2 + test/integration/server.h | 7 +++ test/integration/xds_integration_test.cc | 2 + 22 files changed, 233 insertions(+), 92 deletions(-) create mode 100644 test/config/integration/server_xds.lds.json create mode 100644 test/config/integration/server_xds.rds.json diff --git a/include/envoy/server/BUILD b/include/envoy/server/BUILD index abce7d27e525..bc25419a7558 100644 --- a/include/envoy/server/BUILD +++ b/include/envoy/server/BUILD @@ -111,6 +111,8 @@ envoy_cc_library( "//include/envoy/thread_local:thread_local_interface", "//include/envoy/tracing:http_tracer_interface", "//include/envoy/upstream:cluster_manager_interface", + "//source/common/common:macros", + "//source/common/protobuf", ], ) diff --git a/include/envoy/server/filter_config.h b/include/envoy/server/filter_config.h index 5a56629570fa..7bf92ce9d7ba 100644 --- a/include/envoy/server/filter_config.h +++ b/include/envoy/server/filter_config.h @@ -15,6 +15,9 @@ #include "envoy/tracing/http_tracer.h" #include "envoy/upstream/cluster_manager.h" +#include "common/common/macros.h" +#include "common/protobuf/protobuf.h" + namespace Envoy { namespace Server { @@ -150,6 +153,24 @@ class NamedNetworkFilterConfigFactory { virtual NetworkFilterFactoryCb createFilterFactory(const Json::Object& config, FactoryContext& context) PURE; + /** + * v2 variant of createFilterFactory(..), where filter configs are specified as proto. This may be + * optionally implemented today, but will in the future become compulsory once v1 is deprecated. + */ + virtual NetworkFilterFactoryCb createFilterFactoryFromProto(const Protobuf::Message& config, + FactoryContext& context) { + UNREFERENCED_PARAMETER(config); + UNREFERENCED_PARAMETER(context); + return NetworkFilterFactoryCb(); + } + + /** + * @return std::unique_ptr Empty config proto for v2. This provides sufficient + * type information such that the opaque config can be parsed and placed in the created + * config. Optional today, will be compulsory when v1 is deprecated. + */ + virtual std::unique_ptr createEmptyConfig() { return nullptr; } + /** * @return std::string the identifying name for a particular implementation of a network filter * produced by the factory. @@ -197,6 +218,27 @@ class NamedHttpFilterConfigFactory { const std::string& stat_prefix, FactoryContext& context) PURE; + /** + * v2 API variant of createFilterFactory(..), where filter configs are specified as proto. This + * may be optionally implemented today, but will in the future become compulsory once v1 is + * deprecated. + */ + virtual HttpFilterFactoryCb createFilterFactoryFromProto(const ProtobufWkt::Message& config, + const std::string& stat_prefix, + FactoryContext& context) { + UNREFERENCED_PARAMETER(config); + UNREFERENCED_PARAMETER(stat_prefix); + UNREFERENCED_PARAMETER(context); + return HttpFilterFactoryCb(); + } + + /** + * @return std::unique_ptr Empty config proto for v2. This provides sufficient + * type information such that the opaque config can be parsed and placed in the created + * config. Optional today, will be compulsory when v1 is deprecated. + */ + virtual std::unique_ptr createEmptyConfig() { return nullptr; } + /** * @return std::string the identifying name for a particular implementation of an http filter * produced by the factory. diff --git a/source/common/config/filter_json.cc b/source/common/config/filter_json.cc index 8cda2d473373..99ed03740e36 100644 --- a/source/common/config/filter_json.cc +++ b/source/common/config/filter_json.cc @@ -125,8 +125,10 @@ void FilterJson::translateHttpConnectionManager( JSON_UTIL_SET_STRING(*json_filter, *filter, name); JSON_UTIL_SET_STRING(*json_filter, *filter->mutable_deprecated_v1(), type); - const auto status = Protobuf::util::JsonStringToMessage( - json_filter->getObject("config")->asJsonString(), filter->mutable_config()); + const std::string json_config = "{\"deprecatedV1\": true, \"value\": " + + json_filter->getObject("config")->asJsonString() + "}"; + + const auto status = Protobuf::util::JsonStringToMessage(json_config, filter->mutable_config()); // JSON schema has already validated that this is a valid JSON object. ASSERT(status.ok()); UNREFERENCED_PARAMETER(status); diff --git a/source/common/config/lds_json.cc b/source/common/config/lds_json.cc index 7f0a6fae7855..4ddd06569ca9 100644 --- a/source/common/config/lds_json.cc +++ b/source/common/config/lds_json.cc @@ -28,8 +28,10 @@ void LdsJson::translateListener(const Json::Object& json_listener, JSON_UTIL_SET_STRING(*json_filter, *filter, name); JSON_UTIL_SET_STRING(*json_filter, *filter->mutable_deprecated_v1(), type); - const auto status = Protobuf::util::JsonStringToMessage( - json_filter->getObject("config")->asJsonString(), filter->mutable_config()); + const std::string json_config = "{\"deprecatedV1\": true, \"value\": " + + json_filter->getObject("config")->asJsonString() + "}"; + + const auto status = Protobuf::util::JsonStringToMessage(json_config, filter->mutable_config()); // JSON schema has already validated that this is a valid JSON object. ASSERT(status.ok()); UNREFERENCED_PARAMETER(status); diff --git a/source/common/protobuf/BUILD b/source/common/protobuf/BUILD index da5d5108b20d..c0eb42ecbd64 100644 --- a/source/common/protobuf/BUILD +++ b/source/common/protobuf/BUILD @@ -21,7 +21,9 @@ envoy_cc_library( external_deps = ["protobuf"], deps = [ ":protobuf", + "//source/common/common:assert_lib", "//source/common/common:utility_lib", "//source/common/filesystem:filesystem_lib", + "//source/common/json:json_loader_lib", ], ) diff --git a/source/common/protobuf/protobuf.h b/source/common/protobuf/protobuf.h index 3529abcc4123..6b0eb21ae4a4 100644 --- a/source/common/protobuf/protobuf.h +++ b/source/common/protobuf/protobuf.h @@ -4,8 +4,10 @@ #include #include +#include "google/protobuf/any.pb.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" +#include "google/protobuf/empty.pb.h" #include "google/protobuf/io/coded_stream.h" #include "google/protobuf/io/zero_copy_stream.h" #include "google/protobuf/io/zero_copy_stream_impl_lite.h" diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index 3a5610f18fa1..306933cbbbc5 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -1,6 +1,8 @@ #include "common/protobuf/utility.h" +#include "common/common/assert.h" #include "common/filesystem/filesystem_impl.h" +#include "common/json/json_loader.h" #include "spdlog/spdlog.h" @@ -11,12 +13,24 @@ MissingFieldException::MissingFieldException(const std::string& field_name, : EnvoyException( fmt::format("Field '{}' is missing in: {}", field_name, message.DebugString())) {} -void MessageUtil::loadFromFile(const std::string& path, Protobuf::Message& message) { - const std::string json = Filesystem::fileReadToEnd(path); +void MessageUtil::loadFromJson(const std::string json, Protobuf::Message& message) { const auto status = Protobuf::util::JsonStringToMessage(json, &message); if (!status.ok()) { - throw EnvoyException("Unable to parse JSON as proto: " + json); + throw EnvoyException("Unable to parse JSON as proto (" + status.ToString() + "): " + json); } } +void MessageUtil::loadFromFile(const std::string& path, Protobuf::Message& message) { + loadFromJson(Filesystem::fileReadToEnd(path), message); +} + +Json::ObjectSharedPtr WktUtil::getJsonObjectFromStruct(const Protobuf::Struct& message) { + Protobuf::util::JsonOptions json_options; + ProtobufTypes::String json; + const auto status = Protobuf::util::MessageToJsonString(message, &json, json_options); + // This should always succeed unless something crash-worthy such as out-of-memory. + RELEASE_ASSERT(status.ok()); + return Json::Factory::loadFromString(json); +} + } // namespace Envoy diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index 05c8c13be791..0f1cbf0e2077 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -1,6 +1,7 @@ #pragma once #include "envoy/common/exception.h" +#include "envoy/json/json_object.h" #include "common/common/utility.h" #include "common/protobuf/protobuf.h" @@ -51,7 +52,18 @@ class MessageUtil { return std::hash{}(message.SerializeAsString()); } + static void loadFromJson(const std::string json, Protobuf::Message& message); static void loadFromFile(const std::string& path, Protobuf::Message& message); }; +class WktUtil { +public: + /** + * Extract JSON object from a google.protobuf.Struct. + * @param message message of type type.googleapis.com/google.protobuf.Struct. + * @return Json::ObjectSharedPtr of JSON object or nullptr if unable to extract. + */ + static Json::ObjectSharedPtr getJsonObjectFromStruct(const Protobuf::Struct& message); +}; + } // namespace Envoy diff --git a/source/server/config/network/http_connection_manager.cc b/source/server/config/network/http_connection_manager.cc index a08d4640d23e..f90ad1b658a5 100644 --- a/source/server/config/network/http_connection_manager.cc +++ b/source/server/config/network/http_connection_manager.cc @@ -33,8 +33,11 @@ const std::string HttpConnectionManagerConfig::DEFAULT_SERVER_STRING = "envoy"; SINGLETON_MANAGER_REGISTRATION(date_provider); SINGLETON_MANAGER_REGISTRATION(route_config_provider_manager); -NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactory( - const Json::Object& json_http_connection_manager, FactoryContext& context) { +namespace { + +NetworkFilterFactoryCb createHttpConnectionManagerFilterFactory( + const envoy::api::v2::filter::HttpConnectionManager& http_connection_manager, + FactoryContext& context) { std::shared_ptr date_provider = context.singletonManager().getTyped( SINGLETON_MANAGER_REGISTERED_NAME(date_provider), [&context] { @@ -50,9 +53,6 @@ NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFac context.threadLocal()); }); - envoy::api::v2::filter::HttpConnectionManager http_connection_manager; - Config::FilterJson::translateHttpConnectionManager(json_http_connection_manager, - http_connection_manager); std::shared_ptr http_config(new HttpConnectionManagerConfig( http_connection_manager, context, *date_provider, *route_config_provider_manager)); @@ -67,6 +67,22 @@ NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFac }; } +} // namespace + +NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactory( + const Json::Object& json_http_connection_manager, FactoryContext& context) { + envoy::api::v2::filter::HttpConnectionManager http_connection_manager; + Config::FilterJson::translateHttpConnectionManager(json_http_connection_manager, + http_connection_manager); + return createHttpConnectionManagerFilterFactory(http_connection_manager, context); +} + +NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactoryFromProto( + const Protobuf::Message& config, FactoryContext& context) { + return createHttpConnectionManagerFilterFactory( + dynamic_cast(config), context); +} + /** * Static registration for the HTTP connection manager filter. */ @@ -210,23 +226,26 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( ENVOY_LOG(info, " filter #{}", i); ENVOY_LOG(info, " name: {}", string_name); - Protobuf::util::JsonOptions json_options; - ProtobufTypes::String json_config; - const auto status = - Protobuf::util::MessageToJsonString(proto_config, &json_config, json_options); - // This should always succeed unless something crash-worthy such as out-of-memory. - RELEASE_ASSERT(status.ok()); - UNREFERENCED_PARAMETER(status); - const Json::ObjectSharedPtr filter_config = Json::Factory::loadFromString(json_config); - + const Json::ObjectSharedPtr filter_config = WktUtil::getJsonObjectFromStruct(proto_config); const HttpFilterType type = stringToType(string_type); // Now see if there is a factory that will accept the config. NamedHttpFilterConfigFactory* factory = Registry::FactoryRegistry::getFactory(string_name); if (factory != nullptr) { - HttpFilterFactoryCb callback = - factory->createFilterFactory(*filter_config, stats_prefix_, context_); + HttpFilterFactoryCb callback; + if (filter_config->getBoolean("deprecatedV1", false)) { + callback = factory->createFilterFactory(*filter_config->getObject("value", true), + stats_prefix_, context); + } else { + auto message = factory->createEmptyConfig(); + if (!message) { + throw EnvoyException( + fmt::format("Filter factory for '{}' unexpected proto config", string_name)); + } + MessageUtil::loadFromJson(filter_config->asJsonString(), *message); + callback = factory->createFilterFactoryFromProto(*message, stats_prefix_, context); + } filter_factories_.push_back(callback); } else { // DEPRECATED @@ -234,7 +253,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( bool found_filter = false; for (HttpFilterConfigFactory* config_factory : filterConfigFactories()) { HttpFilterFactoryCb callback = config_factory->tryCreateFilterFactory( - type, string_name, *filter_config, stats_prefix_, context_.server()); + type, string_name, *filter_config->getObject("value", true), stats_prefix_, + context_.server()); if (callback) { filter_factories_.push_back(callback); found_filter = true; @@ -287,7 +307,7 @@ HttpFilterType HttpConnectionManagerConfig::stringToType(const std::string& type } else if (type == "encoder") { return HttpFilterType::Encoder; } else { - ASSERT(type == "both"); + ASSERT(type == "both" || type.empty()); return HttpFilterType::Both; } } diff --git a/source/server/config/network/http_connection_manager.h b/source/server/config/network/http_connection_manager.h index 9463f566e060..9623e12db8e5 100644 --- a/source/server/config/network/http_connection_manager.h +++ b/source/server/config/network/http_connection_manager.h @@ -27,7 +27,13 @@ class HttpConnectionManagerFilterConfigFactory : Logger::Loggable createEmptyConfig() override { + return std::unique_ptr( + new envoy::api::v2::filter::HttpConnectionManager()); + } std::string name() override { return "http_connection_manager"; } NetworkFilterType type() override { return NetworkFilterType::Read; } }; diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 9ea7f17571b4..0d0941a2693b 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -52,7 +52,11 @@ void MainImpl::initialize(const Json::Object& json, const envoy::api::v2::Bootst server.listenerManager().addOrUpdateListener(listener); } - if (json.hasObject("lds")) { + if (bootstrap.has_lds_config()) { + lds_api_.reset(new LdsApi(bootstrap.lds_config(), *cluster_manager_, server.dispatcher(), + server.random(), server.initManager(), server.localInfo(), + server.stats(), server.listenerManager())); + } else if (json.hasObject("lds")) { envoy::api::v2::ConfigSource lds_config; Config::Utility::translateLdsConfig(*json.getObject("lds"), lds_config); lds_api_.reset(new LdsApi(lds_config, *cluster_manager_, server.dispatcher(), server.random(), diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index a5ab77680c93..3bfaee70fe1e 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -25,15 +25,7 @@ ProdListenerComponentFactory::createFilterFactoryList_( const auto& proto_config = filters[i].config(); ENVOY_LOG(info, " filter #{}:", i); ENVOY_LOG(info, " name: {}", string_name); - - Protobuf::util::JsonOptions json_options; - ProtobufTypes::String json_config; - const auto status = - Protobuf::util::MessageToJsonString(proto_config, &json_config, json_options); - // This should always succeed unless something crash-worthy such as out-of-memory. - RELEASE_ASSERT(status.ok()); - UNREFERENCED_PARAMETER(status); - const Json::ObjectSharedPtr filter_config = Json::Factory::loadFromString(json_config); + const Json::ObjectSharedPtr filter_config = WktUtil::getJsonObjectFromStruct(proto_config); // Map filter type string to enum. Configuration::NetworkFilterType type; @@ -42,7 +34,7 @@ ProdListenerComponentFactory::createFilterFactoryList_( } else if (string_type == "write") { type = Configuration::NetworkFilterType::Write; } else { - ASSERT(string_type == "both"); + ASSERT(string_type == "both" || string_type.empty()); type = Configuration::NetworkFilterType::Both; } @@ -51,8 +43,18 @@ ProdListenerComponentFactory::createFilterFactoryList_( Registry::FactoryRegistry::getFactory( string_name); if (factory != nullptr) { - Configuration::NetworkFilterFactoryCb callback = - factory->createFilterFactory(*filter_config, context); + Configuration::NetworkFilterFactoryCb callback; + if (filter_config->getBoolean("deprecatedV1", false)) { + callback = factory->createFilterFactory(*filter_config->getObject("value", true), context); + } else { + auto message = factory->createEmptyConfig(); + if (!message) { + throw EnvoyException( + fmt::format("Filter factory for '{}' unexpected proto config", string_name)); + } + MessageUtil::loadFromJson(filter_config->asJsonString(), *message); + callback = factory->createFilterFactoryFromProto(*message, context); + } ret.push_back(callback); } else { // DEPRECATED @@ -60,8 +62,8 @@ ProdListenerComponentFactory::createFilterFactoryList_( bool found_filter = false; for (Configuration::NetworkFilterConfigFactory* config_factory : Configuration::MainImpl::filterConfigFactories()) { - Configuration::NetworkFilterFactoryCb callback = - config_factory->tryCreateFilterFactory(type, string_name, *filter_config, server); + Configuration::NetworkFilterFactoryCb callback = config_factory->tryCreateFilterFactory( + type, string_name, *filter_config->getObject("value", true), server); if (callback) { ret.push_back(callback); found_filter = true; diff --git a/test/config/integration/BUILD b/test/config/integration/BUILD index 01de7dc841b9..de4519ef3082 100644 --- a/test/config/integration/BUILD +++ b/test/config/integration/BUILD @@ -28,6 +28,8 @@ filegroup( "server_xds.cds.json", "server_xds.eds.json", "server_xds.json", + "server_xds.lds.json", + "server_xds.rds.json", ], ) diff --git a/test/config/integration/server_xds.bootstrap.json b/test/config/integration/server_xds.bootstrap.json index 0b2df22bfb5c..a63c4401ec4f 100644 --- a/test/config/integration/server_xds.bootstrap.json +++ b/test/config/integration/server_xds.bootstrap.json @@ -1,4 +1,7 @@ { + "lds_config": { + "path": "{{ lds_json_path }}" + }, "cds_config": { "path": "{{ cds_json_path }}" } diff --git a/test/config/integration/server_xds.cds.json b/test/config/integration/server_xds.cds.json index b91db5e60f84..1a2adc56b8de 100644 --- a/test/config/integration/server_xds.cds.json +++ b/test/config/integration/server_xds.cds.json @@ -1,18 +1,16 @@ { "versionInfo": "0", - "resources": [ - { - "@type": "type.googleapis.com/envoy.api.v2.Cluster", - "name": "cluster_1", - "connectTimeout": { "seconds": 5 }, - "type": "EDS", - "edsClusterConfig": { - "edsConfig": { - "path": "{{ eds_json_path }}" - } - }, - "lbPolicy": "ROUND_ROBIN", - "http2ProtocolOptions": {} - } - ] + "resources": [{ + "@type": "type.googleapis.com/envoy.api.v2.Cluster", + "name": "cluster_1", + "connectTimeout": { "seconds": 5 }, + "type": "EDS", + "edsClusterConfig": { + "edsConfig": { + "path": "{{ eds_json_path }}" + } + }, + "lbPolicy": "ROUND_ROBIN", + "http2ProtocolOptions": {} + }] } diff --git a/test/config/integration/server_xds.json b/test/config/integration/server_xds.json index f2f9c045e375..111bc3b9d12d 100644 --- a/test/config/integration/server_xds.json +++ b/test/config/integration/server_xds.json @@ -1,39 +1,6 @@ { - "listeners": [ - { - "address": "tcp://{{ ip_loopback_address }}:0", - "filters": [ - { - "type": "read", - "name": "http_connection_manager", - "config": { - "codec_type": "http2", - "drain_timeout_ms": 5000, - "stat_prefix": "router", - "route_config": - { - "virtual_hosts": [ - { - "name": "integration", - "domains": [ "*" ], - "routes": [ - { - "prefix": "/test/long/url", - "cluster": "cluster_1" - } - ] - } - ] - }, - "filters": [ - { "type": "decoder", "name": "router", "config": {}} - ] - } - }] - }], - + "listeners": [], "admin": { "access_log_path": "/dev/null", "address": "tcp://{{ ip_loopback_address }}:0" }, - "cluster_manager": { "clusters": [] } diff --git a/test/config/integration/server_xds.lds.json b/test/config/integration/server_xds.lds.json new file mode 100644 index 000000000000..f10582120654 --- /dev/null +++ b/test/config/integration/server_xds.lds.json @@ -0,0 +1,28 @@ +{ + "versionInfo": "0", + "resources": [{ + "@type": "type.googleapis.com/envoy.api.v2.Listener", + "name": "listener_0", + "address": { + "socketAddress": { + "address": "{{ ntop_ip_loopback_address }}", + "portValue": 0 + } + }, + "filterChains": [{ + "filters": [{ + "name": "http_connection_manager", + "config": { + "codecType": "HTTP2", + "drainTimeout": "5s", + "statPrefix": "router", + "rds": { + "routeConfigName": "route_config_0", + "configSource": { "path": "{{ rds_json_path }}" } + }, + "httpFilters": [{ "name": "router", "config": {"deprecatedV1": true} }] + } + }] + }] + }] +} diff --git a/test/config/integration/server_xds.rds.json b/test/config/integration/server_xds.rds.json new file mode 100644 index 000000000000..63147a998ddb --- /dev/null +++ b/test/config/integration/server_xds.rds.json @@ -0,0 +1,15 @@ +{ + "versionInfo": "0", + "resources": [{ + "@type": "type.googleapis.com/envoy.api.v2.RouteConfiguration", + "name": "route_config_0", + "virtual_hosts": [{ + "name": "integration", + "domains": [ "*" ], + "routes": [{ + "match": { "prefix": "/test/long/url" }, + "route": { "cluster": "cluster_1" } + }] + }] + }] +} diff --git a/test/integration/integration.cc b/test/integration/integration.cc index beb955acccb5..a983b23702c1 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -332,12 +332,19 @@ void BaseIntegrationTest::createApiTestServer(const std::string& json_path, api_filesystem_config.eds_path_, port_map_, version_); const std::string cds_path = TestEnvironment::temporaryFileSubstitute( api_filesystem_config.cds_path_, {{"eds_json_path", eds_path}}, port_map_, version_); + const std::string rds_path = TestEnvironment::temporaryFileSubstitute( + api_filesystem_config.rds_path_, port_map_, version_); + const std::string lds_path = TestEnvironment::temporaryFileSubstitute( + api_filesystem_config.lds_path_, {{"rds_json_path", rds_path}}, port_map_, version_); test_server_ = IntegrationTestServer::create( TestEnvironment::temporaryFileSubstitute(json_path, port_map_, version_), - TestEnvironment::temporaryFileSubstitute(api_filesystem_config.bootstrap_path_, - {{"cds_json_path", cds_path}}, port_map_, - version_), + TestEnvironment::temporaryFileSubstitute( + api_filesystem_config.bootstrap_path_, + {{"cds_json_path", cds_path}, {"lds_json_path", lds_path}}, port_map_, version_), version_); + // Need to ensure we have an LDS update before invoking registerTestServerPorts() below that + // needs to know about the bound listener ports. + test_server_->waitForCounterGe("listener_manager.lds.update_success", 1); } registerTestServerPorts(port_names); } diff --git a/test/integration/integration.h b/test/integration/integration.h index 2ac76ab7d983..e7c74a360fb6 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -155,6 +155,8 @@ struct ApiFilesystemConfig { std::string bootstrap_path_; std::string cds_path_; std::string eds_path_; + std::string lds_path_; + std::string rds_path_; }; /** diff --git a/test/integration/server.h b/test/integration/server.h index a3b3bbef2105..d40932a527a5 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -207,6 +207,13 @@ class IntegrationTestServer : Logger::Loggable, } void start(const Network::Address::IpVersion version); void start(); + + void waitForCounterGe(const std::string& name, uint64_t value) { + while (counter(name)->value() < value) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + } + Stats::CounterSharedPtr counter(const std::string& name) { // When using the thread local store, only counters() is thread safe. This also allows us // to test if a counter exists at all versus just defaulting to zero. diff --git a/test/integration/xds_integration_test.cc b/test/integration/xds_integration_test.cc index c05aef0bc5d8..636220a30c40 100644 --- a/test/integration/xds_integration_test.cc +++ b/test/integration/xds_integration_test.cc @@ -20,6 +20,8 @@ class XdsIntegrationTest : public BaseIntegrationTest, .bootstrap_path_ = "test/config/integration/server_xds.bootstrap.json", .cds_path_ = "test/config/integration/server_xds.cds.json", .eds_path_ = "test/config/integration/server_xds.eds.json", + .lds_path_ = "test/config/integration/server_xds.lds.json", + .rds_path_ = "test/config/integration/server_xds.rds.json", }, {"http"}); } From 03747c0ebf0c7bc4581e63507736c3d48516556d Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Sun, 20 Aug 2017 07:50:43 -0400 Subject: [PATCH 2/7] Fix race-to-listen/connect, consolidate waitForCountGe-like code. --- include/envoy/server/filter_config.h | 16 ++++++++-------- .../config/network/http_connection_manager.cc | 4 ++-- .../config/network/http_connection_manager.h | 2 +- source/server/listener_manager_impl.cc | 7 +++++-- source/server/listener_manager_impl.h | 1 + test/integration/integration.cc | 2 +- test/integration/ratelimit_integration_test.cc | 9 +-------- test/integration/server.h | 10 ++++++++++ 8 files changed, 29 insertions(+), 22 deletions(-) diff --git a/include/envoy/server/filter_config.h b/include/envoy/server/filter_config.h index 7bf92ce9d7ba..a7af7023cf0b 100644 --- a/include/envoy/server/filter_config.h +++ b/include/envoy/server/filter_config.h @@ -165,11 +165,11 @@ class NamedNetworkFilterConfigFactory { } /** - * @return std::unique_ptr Empty config proto for v2. This provides sufficient - * type information such that the opaque config can be parsed and placed in the created - * config. Optional today, will be compulsory when v1 is deprecated. + * @return std::unique_ptr create empty config proto message for v2. The filter + * config will be parsed into the result of this method. Optional today, will be + * compulsory when v1 is deprecated. */ - virtual std::unique_ptr createEmptyConfig() { return nullptr; } + virtual std::unique_ptr createConfigProto() { return nullptr; } /** * @return std::string the identifying name for a particular implementation of a network filter @@ -233,11 +233,11 @@ class NamedHttpFilterConfigFactory { } /** - * @return std::unique_ptr Empty config proto for v2. This provides sufficient - * type information such that the opaque config can be parsed and placed in the created - * config. Optional today, will be compulsory when v1 is deprecated. + * @return std::unique_ptr create empty config proto message for v2. The filter + * config will be parsed into the result of this method. Optional today, will be + * compulsory when v1 is deprecated. */ - virtual std::unique_ptr createEmptyConfig() { return nullptr; } + virtual std::unique_ptr createConfigProto() { return nullptr; } /** * @return std::string the identifying name for a particular implementation of an http filter diff --git a/source/server/config/network/http_connection_manager.cc b/source/server/config/network/http_connection_manager.cc index f90ad1b658a5..ff255a41d405 100644 --- a/source/server/config/network/http_connection_manager.cc +++ b/source/server/config/network/http_connection_manager.cc @@ -238,10 +238,10 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( callback = factory->createFilterFactory(*filter_config->getObject("value", true), stats_prefix_, context); } else { - auto message = factory->createEmptyConfig(); + auto message = factory->createConfigProto(); if (!message) { throw EnvoyException( - fmt::format("Filter factory for '{}' unexpected proto config", string_name)); + fmt::format("Filter factory for '{}' has unexpected proto config", string_name)); } MessageUtil::loadFromJson(filter_config->asJsonString(), *message); callback = factory->createFilterFactoryFromProto(*message, stats_prefix_, context); diff --git a/source/server/config/network/http_connection_manager.h b/source/server/config/network/http_connection_manager.h index 9623e12db8e5..36fdbc7bb616 100644 --- a/source/server/config/network/http_connection_manager.h +++ b/source/server/config/network/http_connection_manager.h @@ -30,7 +30,7 @@ class HttpConnectionManagerFilterConfigFactory : Logger::Loggable createEmptyConfig() override { + std::unique_ptr createConfigProto() override { return std::unique_ptr( new envoy::api::v2::filter::HttpConnectionManager()); } diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 3bfaee70fe1e..b2d1fd539741 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -47,10 +47,10 @@ ProdListenerComponentFactory::createFilterFactoryList_( if (filter_config->getBoolean("deprecatedV1", false)) { callback = factory->createFilterFactory(*filter_config->getObject("value", true), context); } else { - auto message = factory->createEmptyConfig(); + auto message = factory->createConfigProto(); if (!message) { throw EnvoyException( - fmt::format("Filter factory for '{}' unexpected proto config", string_name)); + fmt::format("Filter factory for '{}' has unexpected proto config", string_name)); } MessageUtil::loadFromJson(filter_config->asJsonString(), *message); callback = factory->createFilterFactoryFromProto(*message, context); @@ -414,6 +414,9 @@ void ListenerManagerImpl::addListenerToWorker(Worker& worker, ListenerImpl& list stats_.listener_create_failure_.inc(); removeListener(listener.name()); } + if (success) { + stats_.listener_create_success_.inc(); + } }); }); } diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index c70fc252892f..bb675380d947 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -58,6 +58,7 @@ typedef std::unique_ptr ListenerImplPtr; COUNTER(listener_added) \ COUNTER(listener_modified) \ COUNTER(listener_removed) \ + COUNTER(listener_create_success) \ COUNTER(listener_create_failure) \ GAUGE (total_listeners_warming) \ GAUGE (total_listeners_active) \ diff --git a/test/integration/integration.cc b/test/integration/integration.cc index a983b23702c1..1c4a7ad6bc46 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -344,7 +344,7 @@ void BaseIntegrationTest::createApiTestServer(const std::string& json_path, version_); // Need to ensure we have an LDS update before invoking registerTestServerPorts() below that // needs to know about the bound listener ports. - test_server_->waitForCounterGe("listener_manager.lds.update_success", 1); + test_server_->waitForCounterGe("listener_manager.listener_create_success", 1); } registerTestServerPorts(port_names); } diff --git a/test/integration/ratelimit_integration_test.cc b/test/integration/ratelimit_integration_test.cc index f870fae6fe6a..03950d13f934 100644 --- a/test/integration/ratelimit_integration_test.cc +++ b/test/integration/ratelimit_integration_test.cc @@ -165,14 +165,7 @@ TEST_P(RatelimitIntegrationTest, Error) { TEST_P(RatelimitIntegrationTest, Timeout) { initiateClientConnection(); waitForRatelimitRequest(); - // Keep polling stats until the HTTP ratelimit wait times out. - const uint32_t sleep_ms = 100; - for (int32_t timeout_wait_ms = 50000; timeout_wait_ms > 0; timeout_wait_ms -= sleep_ms) { - if (test_server_->counter("cluster.ratelimit.upstream_rq_timeout")->value() > 0) { - break; - } - std::this_thread::sleep_for(std::chrono::milliseconds(sleep_ms)); - } + test_server_->waitForCounterGe("cluster.ratelimit.upstream_rq_timeout", 1); // Rate limiter fails open waitForSuccessfulUpstreamResponse(); cleanup(); diff --git a/test/integration/server.h b/test/integration/server.h index d40932a527a5..7e8425883ff2 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -214,12 +214,22 @@ class IntegrationTestServer : Logger::Loggable, } } + void waitForGaugeGe(const std::string& name, uint64_t value) { + while (gauge(name)->value() < value) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + } + Stats::CounterSharedPtr counter(const std::string& name) { // When using the thread local store, only counters() is thread safe. This also allows us // to test if a counter exists at all versus just defaulting to zero. return TestUtility::findCounter(*stat_store_, name); } + Stats::GaugeSharedPtr gauge(const std::string& name) { + return TestUtility::findGauge(*stat_store_, name); + } + // TestHooks void onWorkerListenerAdded() override; void onWorkerListenerRemoved() override; From b705be97b0c755b24655ddd685bc947f202c77e7 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Mon, 21 Aug 2017 16:16:46 -0400 Subject: [PATCH 3/7] Fix test flakes by making protobuf serialization deterministic. --- source/common/protobuf/utility.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index 0f1cbf0e2077..9937fc7d448f 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -49,7 +49,12 @@ class RepeatedPtrUtil { class MessageUtil { public: static std::size_t hash(const Protobuf::Message& message) { - return std::hash{}(message.SerializeAsString()); + std::string text; + Protobuf::io::StringOutputStream string_stream(&text); + Protobuf::io::CodedOutputStream coded_stream(&string_stream); + coded_stream.SetSerializationDeterministic(true); + message.SerializeToCodedStream(&coded_stream); + return std::hash{}(text); } static void loadFromJson(const std::string json, Protobuf::Message& message); From 1b5268316561483654c1be3094d06e30365732ed Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Mon, 21 Aug 2017 18:36:36 -0400 Subject: [PATCH 4/7] Review feedback. --- docs/configuration/listeners/listeners.rst | 2 ++ source/common/protobuf/utility.cc | 2 +- source/common/protobuf/utility.h | 4 +++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/configuration/listeners/listeners.rst b/docs/configuration/listeners/listeners.rst index dbaed6aa63ea..1673d9604ea6 100644 --- a/docs/configuration/listeners/listeners.rst +++ b/docs/configuration/listeners/listeners.rst @@ -90,6 +90,8 @@ statistics: listener_added, Counter, Total listeners added (either via static config or LDS) listener_modified, Counter, Total listeners modified (via LDS) listener_removed, Counter, Total listeners removed (via LDS) + listener_create_success, Counter, Total listener objects successfully added to workers. + listener_create_failure, Counter, Total failed listener object additions to workers. total_listeners_warming, Gauge, Number of currently warming listeners total_listeners_active, Gauge, Number of currently active listeners total_listeners_draining, Gauge, Number of currently draining listeners diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index 306933cbbbc5..80a01c1964bd 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -13,7 +13,7 @@ MissingFieldException::MissingFieldException(const std::string& field_name, : EnvoyException( fmt::format("Field '{}' is missing in: {}", field_name, message.DebugString())) {} -void MessageUtil::loadFromJson(const std::string json, Protobuf::Message& message) { +void MessageUtil::loadFromJson(const std::string& json, Protobuf::Message& message) { const auto status = Protobuf::util::JsonStringToMessage(json, &message); if (!status.ok()) { throw EnvoyException("Unable to parse JSON as proto (" + status.ToString() + "): " + json); diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index 9937fc7d448f..c7af1cc14b7c 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -49,6 +49,8 @@ class RepeatedPtrUtil { class MessageUtil { public: static std::size_t hash(const Protobuf::Message& message) { + // Use Protobuf::io::CodedOutputStream to force deterministic serialization, so that the same + // message doesn't hash to different values. std::string text; Protobuf::io::StringOutputStream string_stream(&text); Protobuf::io::CodedOutputStream coded_stream(&string_stream); @@ -57,7 +59,7 @@ class MessageUtil { return std::hash{}(text); } - static void loadFromJson(const std::string json, Protobuf::Message& message); + static void loadFromJson(const std::string& json, Protobuf::Message& message); static void loadFromFile(const std::string& path, Protobuf::Message& message); }; From ab0c25c4d6554b6097938ff326d5373ced8d6349 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Mon, 21 Aug 2017 19:14:31 -0400 Subject: [PATCH 5/7] LDS/RDS xds_integration_test JSON -> YAML. --- test/config/integration/BUILD | 4 +-- test/config/integration/server_xds.lds.json | 28 --------------------- test/config/integration/server_xds.lds.yaml | 19 ++++++++++++++ test/config/integration/server_xds.rds.json | 15 ----------- test/config/integration/server_xds.rds.yaml | 10 ++++++++ 5 files changed, 31 insertions(+), 45 deletions(-) delete mode 100644 test/config/integration/server_xds.lds.json create mode 100644 test/config/integration/server_xds.lds.yaml delete mode 100644 test/config/integration/server_xds.rds.json create mode 100644 test/config/integration/server_xds.rds.yaml diff --git a/test/config/integration/BUILD b/test/config/integration/BUILD index 610d0645e57c..2bf981845835 100644 --- a/test/config/integration/BUILD +++ b/test/config/integration/BUILD @@ -27,9 +27,9 @@ filegroup( "server_xds.bootstrap.yaml", "server_xds.cds.yaml", "server_xds.eds.yaml", + "server_xds.lds.yaml", + "server_xds.rds.yaml", "server_xds.json", - "server_xds.lds.json", - "server_xds.rds.json", ], ) diff --git a/test/config/integration/server_xds.lds.json b/test/config/integration/server_xds.lds.json deleted file mode 100644 index f10582120654..000000000000 --- a/test/config/integration/server_xds.lds.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "versionInfo": "0", - "resources": [{ - "@type": "type.googleapis.com/envoy.api.v2.Listener", - "name": "listener_0", - "address": { - "socketAddress": { - "address": "{{ ntop_ip_loopback_address }}", - "portValue": 0 - } - }, - "filterChains": [{ - "filters": [{ - "name": "http_connection_manager", - "config": { - "codecType": "HTTP2", - "drainTimeout": "5s", - "statPrefix": "router", - "rds": { - "routeConfigName": "route_config_0", - "configSource": { "path": "{{ rds_json_path }}" } - }, - "httpFilters": [{ "name": "router", "config": {"deprecatedV1": true} }] - } - }] - }] - }] -} diff --git a/test/config/integration/server_xds.lds.yaml b/test/config/integration/server_xds.lds.yaml new file mode 100644 index 000000000000..48c836b5a805 --- /dev/null +++ b/test/config/integration/server_xds.lds.yaml @@ -0,0 +1,19 @@ +versionInfo: "0" +resources: +- "@type": type.googleapis.com/envoy.api.v2.Listener + name: listener_0 + address: + socketAddress: + address: {{ ntop_ip_loopback_address }} + portValue: 0 + filterChains: + - filters: + - name: http_connection_manager + config: + codecType: HTTP2 + drainTimeout: 5s + statPrefix: router + rds: + routeConfigName: route_config_0 + configSource: { path: {{ rds_json_path }} } + httpFilters: [{ name: router, config: { deprecatedV1: true }}] diff --git a/test/config/integration/server_xds.rds.json b/test/config/integration/server_xds.rds.json deleted file mode 100644 index 63147a998ddb..000000000000 --- a/test/config/integration/server_xds.rds.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "versionInfo": "0", - "resources": [{ - "@type": "type.googleapis.com/envoy.api.v2.RouteConfiguration", - "name": "route_config_0", - "virtual_hosts": [{ - "name": "integration", - "domains": [ "*" ], - "routes": [{ - "match": { "prefix": "/test/long/url" }, - "route": { "cluster": "cluster_1" } - }] - }] - }] -} diff --git a/test/config/integration/server_xds.rds.yaml b/test/config/integration/server_xds.rds.yaml new file mode 100644 index 000000000000..252b2b46395b --- /dev/null +++ b/test/config/integration/server_xds.rds.yaml @@ -0,0 +1,10 @@ +versionInfo: "0" +resources: +- "@type": type.googleapis.com/envoy.api.v2.RouteConfiguration + name: route_config_0 + virtual_hosts: + - name: integration + domains: [ "*" ] + routes: + - match: { prefix: "/test/long/url" } + route: { cluster: cluster_1 } From 9079ccc0a7f2d9b4b71a1cfa6f1117ece8fd8166 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Mon, 21 Aug 2017 21:46:37 -0400 Subject: [PATCH 6/7] Review feedback, fix format. --- include/envoy/server/filter_config.h | 8 ++++---- source/common/protobuf/protobuf.h | 3 ++- source/server/config/network/http_connection_manager.h | 2 +- test/config/integration/BUILD | 2 +- test/integration/server.h | 2 ++ 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/include/envoy/server/filter_config.h b/include/envoy/server/filter_config.h index a7af7023cf0b..33b11eae5aa7 100644 --- a/include/envoy/server/filter_config.h +++ b/include/envoy/server/filter_config.h @@ -165,11 +165,11 @@ class NamedNetworkFilterConfigFactory { } /** - * @return std::unique_ptr create empty config proto message for v2. The filter + * @return ProtobufTypes::MessagePtr create empty config proto message for v2. The filter * config will be parsed into the result of this method. Optional today, will be * compulsory when v1 is deprecated. */ - virtual std::unique_ptr createConfigProto() { return nullptr; } + virtual ProtobufTypes::MessagePtr createConfigProto() { return nullptr; } /** * @return std::string the identifying name for a particular implementation of a network filter @@ -233,11 +233,11 @@ class NamedHttpFilterConfigFactory { } /** - * @return std::unique_ptr create empty config proto message for v2. The filter + * @return ProtobufTypes::MessagePtr create empty config proto message for v2. The filter * config will be parsed into the result of this method. Optional today, will be * compulsory when v1 is deprecated. */ - virtual std::unique_ptr createConfigProto() { return nullptr; } + virtual ProtobufTypes::MessagePtr createConfigProto() { return nullptr; } /** * @return std::string the identifying name for a particular implementation of an http filter diff --git a/source/common/protobuf/protobuf.h b/source/common/protobuf/protobuf.h index 6b0eb21ae4a4..0622a5250878 100644 --- a/source/common/protobuf/protobuf.h +++ b/source/common/protobuf/protobuf.h @@ -4,7 +4,6 @@ #include #include -#include "google/protobuf/any.pb.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/empty.pb.h" @@ -44,6 +43,8 @@ namespace ProtobufWkt = google::protobuf; // import. namespace ProtobufTypes { +typedef std::unique_ptr MessagePtr; + typedef std::string String; typedef int64_t Int64; diff --git a/source/server/config/network/http_connection_manager.h b/source/server/config/network/http_connection_manager.h index 36fdbc7bb616..f31b6f8ba638 100644 --- a/source/server/config/network/http_connection_manager.h +++ b/source/server/config/network/http_connection_manager.h @@ -30,7 +30,7 @@ class HttpConnectionManagerFilterConfigFactory : Logger::Loggable createConfigProto() override { + ProtobufTypes::MessagePtr createConfigProto() override { return std::unique_ptr( new envoy::api::v2::filter::HttpConnectionManager()); } diff --git a/test/config/integration/BUILD b/test/config/integration/BUILD index 2bf981845835..51bdc3b92d71 100644 --- a/test/config/integration/BUILD +++ b/test/config/integration/BUILD @@ -27,9 +27,9 @@ filegroup( "server_xds.bootstrap.yaml", "server_xds.cds.yaml", "server_xds.eds.yaml", + "server_xds.json", "server_xds.lds.yaml", "server_xds.rds.yaml", - "server_xds.json", ], ) diff --git a/test/integration/server.h b/test/integration/server.h index 7e8425883ff2..5f063443f436 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -227,6 +227,8 @@ class IntegrationTestServer : Logger::Loggable, } Stats::GaugeSharedPtr gauge(const std::string& name) { + // When using the thread local store, only gauges() is thread safe. This also allows us + // to test if a counter exists at all versus just defaulting to zero. return TestUtility::findGauge(*stat_store_, name); } From 51a7f606514b8dc9cce66bd6b786693e7bd7e5f2 Mon Sep 17 00:00:00 2001 From: Harvey Tuch Date: Tue, 22 Aug 2017 11:14:25 -0400 Subject: [PATCH 7/7] Rename to createEmptyConfigProto(), better comments. --- include/envoy/server/filter_config.h | 14 ++++++++------ .../config/network/http_connection_manager.cc | 2 +- .../config/network/http_connection_manager.h | 2 +- source/server/listener_manager_impl.cc | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/include/envoy/server/filter_config.h b/include/envoy/server/filter_config.h index 33b11eae5aa7..086076325a01 100644 --- a/include/envoy/server/filter_config.h +++ b/include/envoy/server/filter_config.h @@ -166,10 +166,11 @@ class NamedNetworkFilterConfigFactory { /** * @return ProtobufTypes::MessagePtr create empty config proto message for v2. The filter - * config will be parsed into the result of this method. Optional today, will be - * compulsory when v1 is deprecated. + * config, which arrives in an opaque google.protobuf.Struct message, will be converted to + * JSON and then parsed into this empty proto. Optional today, will be compulsory when v1 + * is deprecated. */ - virtual ProtobufTypes::MessagePtr createConfigProto() { return nullptr; } + virtual ProtobufTypes::MessagePtr createEmptyConfigProto() { return nullptr; } /** * @return std::string the identifying name for a particular implementation of a network filter @@ -234,10 +235,11 @@ class NamedHttpFilterConfigFactory { /** * @return ProtobufTypes::MessagePtr create empty config proto message for v2. The filter - * config will be parsed into the result of this method. Optional today, will be - * compulsory when v1 is deprecated. + * config, which arrives in an opaque google.protobuf.Struct message, will be converted to + * JSON and then parsed into this empty proto. Optional today, will be compulsory when v1 + * is deprecated. */ - virtual ProtobufTypes::MessagePtr createConfigProto() { return nullptr; } + virtual ProtobufTypes::MessagePtr createEmptyConfigProto() { return nullptr; } /** * @return std::string the identifying name for a particular implementation of an http filter diff --git a/source/server/config/network/http_connection_manager.cc b/source/server/config/network/http_connection_manager.cc index ff255a41d405..3ae7a96f61e3 100644 --- a/source/server/config/network/http_connection_manager.cc +++ b/source/server/config/network/http_connection_manager.cc @@ -238,7 +238,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( callback = factory->createFilterFactory(*filter_config->getObject("value", true), stats_prefix_, context); } else { - auto message = factory->createConfigProto(); + auto message = factory->createEmptyConfigProto(); if (!message) { throw EnvoyException( fmt::format("Filter factory for '{}' has unexpected proto config", string_name)); diff --git a/source/server/config/network/http_connection_manager.h b/source/server/config/network/http_connection_manager.h index f31b6f8ba638..738b92d0ca20 100644 --- a/source/server/config/network/http_connection_manager.h +++ b/source/server/config/network/http_connection_manager.h @@ -30,7 +30,7 @@ class HttpConnectionManagerFilterConfigFactory : Logger::Loggable( new envoy::api::v2::filter::HttpConnectionManager()); } diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index b2d1fd539741..8241cf0078d6 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -47,7 +47,7 @@ ProdListenerComponentFactory::createFilterFactoryList_( if (filter_config->getBoolean("deprecatedV1", false)) { callback = factory->createFilterFactory(*filter_config->getObject("value", true), context); } else { - auto message = factory->createConfigProto(); + auto message = factory->createEmptyConfigProto(); if (!message) { throw EnvoyException( fmt::format("Filter factory for '{}' has unexpected proto config", string_name));