From 21c5e5f3dd48a2d6968b82d589b356cc9052dcc6 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Thu, 17 Dec 2020 10:50:16 -0500 Subject: [PATCH 01/16] initial fix Signed-off-by: Yuval Kohavi --- .../envoy/server/bootstrap_extension_config.h | 5 ++- source/common/network/socket_interface.h | 3 +- .../common/network/socket_interface_impl.cc | 2 +- source/common/network/socket_interface_impl.h | 3 +- source/extensions/bootstrap/wasm/config.cc | 38 ++++++++----------- source/extensions/bootstrap/wasm/config.h | 22 +++++------ source/server/server.cc | 12 ++++-- test/extensions/bootstrap/wasm/config_test.cc | 4 +- .../server/bootstrap_extension_factory.h | 3 +- 9 files changed, 50 insertions(+), 42 deletions(-) diff --git a/include/envoy/server/bootstrap_extension_config.h b/include/envoy/server/bootstrap_extension_config.h index 7eaf4dcb25302..2c8abc1bd46b2 100644 --- a/include/envoy/server/bootstrap_extension_config.h +++ b/include/envoy/server/bootstrap_extension_config.h @@ -15,6 +15,9 @@ namespace Server { class BootstrapExtension { public: virtual ~BootstrapExtension() = default; + + // Called when server is done initializing and we have the ServerFactoryConext available. + virtual void serverInitialized(Configuration::ServerFactoryContext& context) PURE; }; using BootstrapExtensionPtr = std::unique_ptr; @@ -37,7 +40,7 @@ class BootstrapExtensionFactory : public Config::TypedFactory { * @param context general filter context through which persistent resources can be accessed. */ virtual BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config, - ServerFactoryContext& context) PURE; + ProtobufMessage::ValidationVisitor& validation_visitor) PURE; std::string category() const override { return "envoy.bootstrap"; } }; diff --git a/source/common/network/socket_interface.h b/source/common/network/socket_interface.h index 070dec2b3c698..49d517b0bdd34 100644 --- a/source/common/network/socket_interface.h +++ b/source/common/network/socket_interface.h @@ -17,7 +17,8 @@ namespace Network { class SocketInterfaceExtension : public Server::BootstrapExtension { public: SocketInterfaceExtension(SocketInterface& sock_interface) : sock_interface_(sock_interface) {} - + // Server::BootstrapExtension + void serverInitialized(Server::Configuration::ServerFactoryContext&) override {} protected: SocketInterface& sock_interface_; }; diff --git a/source/common/network/socket_interface_impl.cc b/source/common/network/socket_interface_impl.cc index 8a3802e36769e..48f32278bcb21 100644 --- a/source/common/network/socket_interface_impl.cc +++ b/source/common/network/socket_interface_impl.cc @@ -91,7 +91,7 @@ bool SocketInterfaceImpl::ipFamilySupported(int domain) { Server::BootstrapExtensionPtr SocketInterfaceImpl::createBootstrapExtension(const Protobuf::Message&, - Server::Configuration::ServerFactoryContext&) { + ProtobufMessage::ValidationVisitor&) { return std::make_unique(*this); } diff --git a/source/common/network/socket_interface_impl.h b/source/common/network/socket_interface_impl.h index d87ad6a959130..7220b924deb86 100644 --- a/source/common/network/socket_interface_impl.h +++ b/source/common/network/socket_interface_impl.h @@ -19,7 +19,8 @@ class SocketInterfaceImpl : public SocketInterfaceBase { // Server::Configuration::BootstrapExtensionFactory Server::BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config, - Server::Configuration::ServerFactoryContext& context) override; + ProtobufMessage::ValidationVisitor& validation_visitor) override; + ProtobufTypes::MessagePtr createEmptyConfigProto() override; std::string name() const override { return "envoy.extensions.network.socket_interface.default_socket_interface"; diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index 0e8f4caa99ac9..997a2d193f80d 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -14,19 +14,18 @@ namespace Extensions { namespace Bootstrap { namespace Wasm { -static const std::string INLINE_STRING = ""; +void WasmServiceExtension::serverInitialized(Server::Configuration::ServerFactoryContext& context) { + createWasm(context); +} -void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& config, - Server::Configuration::ServerFactoryContext& context, - CreateWasmServiceCallback&& cb) { +void WasmServiceExtension::createWasm(Server::Configuration::ServerFactoryContext& context) { auto plugin = std::make_shared( - config.config().name(), config.config().root_id(), config.config().vm_config().vm_id(), - config.config().vm_config().runtime(), - Common::Wasm::anyToBytes(config.config().configuration()), config.config().fail_open(), + config_.config().name(), config_.config().root_id(), config_.config().vm_config().vm_id(), + config_.config().vm_config().runtime(), + Common::Wasm::anyToBytes(config_.config().configuration()), config_.config().fail_open(), envoy::config::core::v3::TrafficDirection::UNSPECIFIED, context.localInfo(), nullptr); - bool singleton = config.singleton(); - auto callback = [&context, singleton, plugin, cb](Common::Wasm::WasmHandleSharedPtr base_wasm) { + auto callback = [this, &context, plugin](Common::Wasm::WasmHandleSharedPtr base_wasm) { if (!base_wasm) { if (plugin->fail_open_) { ENVOY_LOG(error, "Unable to create Wasm service {}", plugin->name_); @@ -35,10 +34,10 @@ void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& con } return; } - if (singleton) { + if (config_.singleton()) { // Return a Wasm VM which will be stored as a singleton by the Server. - cb(std::make_unique(plugin, Common::Wasm::getOrCreateThreadLocalPlugin( - base_wasm, plugin, context.dispatcher()))); + wasm_service_ = std::make_unique(plugin, Common::Wasm::getOrCreateThreadLocalPlugin( + base_wasm, plugin, context.dispatcher())); return; } // Per-thread WASM VM. @@ -48,11 +47,11 @@ void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& con tls_slot->set([base_wasm, plugin](Event::Dispatcher& dispatcher) { return Common::Wasm::getOrCreateThreadLocalPlugin(base_wasm, plugin, dispatcher); }); - cb(std::make_unique(plugin, std::move(tls_slot))); + wasm_service_ = std::make_unique(plugin, std::move(tls_slot)); }; if (!Common::Wasm::createWasm( - config.config().vm_config(), plugin, context.scope().createScope(""), + config_.config().vm_config(), plugin, context.scope().createScope(""), context.clusterManager(), context.initManager(), context.dispatcher(), context.api(), context.lifecycleNotifier(), remote_data_provider_, std::move(callback))) { // NB: throw if we get a synchronous configuration failures as this is how such failures are @@ -64,17 +63,12 @@ void WasmFactory::createWasm(const envoy::extensions::wasm::v3::WasmService& con Server::BootstrapExtensionPtr WasmFactory::createBootstrapExtension(const Protobuf::Message& config, - Server::Configuration::ServerFactoryContext& context) { + ProtobufMessage::ValidationVisitor& validation_visitor) { auto typed_config = MessageUtil::downcastAndValidate( - config, context.messageValidationContext().staticValidationVisitor()); + config, validation_visitor); - auto wasm_service_extension = std::make_unique(); - createWasm(typed_config, context, - [extension = wasm_service_extension.get()](WasmServicePtr wasm) { - extension->wasm_service_ = std::move(wasm); - }); - return wasm_service_extension; + return std::make_unique(typed_config); } // /** diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index b8f3850ef6212..75fb19480cf4a 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -34,37 +34,37 @@ class WasmService { }; using WasmServicePtr = std::unique_ptr; -using CreateWasmServiceCallback = std::function; -class WasmFactory : public Server::Configuration::BootstrapExtensionFactory, - Logger::Loggable { +class WasmFactory : public Server::Configuration::BootstrapExtensionFactory { public: ~WasmFactory() override = default; std::string name() const override { return "envoy.bootstrap.wasm"; } - void createWasm(const envoy::extensions::wasm::v3::WasmService& config, - Server::Configuration::ServerFactoryContext& context, - CreateWasmServiceCallback&& cb); Server::BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config, - Server::Configuration::ServerFactoryContext& context) override; + ProtobufMessage::ValidationVisitor& validation_visitor) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique(); } -private: - Config::DataSource::RemoteAsyncDataProviderPtr remote_data_provider_; }; -class WasmServiceExtension : public Server::BootstrapExtension { +class WasmServiceExtension : public Server::BootstrapExtension, + Logger::Loggable { public: + WasmServiceExtension(const envoy::extensions::wasm::v3::WasmService& config) : config_(config) {} WasmService& wasmService() { ASSERT(wasm_service_ != nullptr); return *wasm_service_; } + void serverInitialized(Server::Configuration::ServerFactoryContext& context) override; private: + + void createWasm(Server::Configuration::ServerFactoryContext& context); + + envoy::extensions::wasm::v3::WasmService config_; WasmServicePtr wasm_service_; - friend class WasmFactory; + Config::DataSource::RemoteAsyncDataProviderPtr remote_data_provider_; }; } // namespace Wasm diff --git a/source/server/server.cc b/source/server/server.cc index ef9e211c2e8fe..f70569481a2c6 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -139,7 +139,10 @@ InstanceImpl::~InstanceImpl() { ENVOY_LOG(debug, "destroyed listener manager"); } -Upstream::ClusterManager& InstanceImpl::clusterManager() { return *config_.clusterManager(); } +Upstream::ClusterManager& InstanceImpl::clusterManager() { + ASSERT(config_.clusterManager() != nullptr); + return *config_.clusterManager(); +} void InstanceImpl::drainListeners() { ENVOY_LOG(info, "closing and draining listeners"); @@ -428,8 +431,7 @@ void InstanceImpl::initialize(const Options& options, auto config = Config::Utility::translateAnyToFactoryConfig( bootstrap_extension.typed_config(), messageValidationContext().staticValidationVisitor(), factory); - bootstrap_extensions_.push_back( - factory.createBootstrapExtension(*config, serverFactoryContext())); + bootstrap_extensions_.push_back(factory.createBootstrapExtension(*config, messageValidationContext().staticValidationVisitor())); } // Register the fatal actions. @@ -534,6 +536,10 @@ void InstanceImpl::initialize(const Options& options, // cluster_manager_factory_ is available. config_.initialize(bootstrap_, *this, *cluster_manager_factory_); + for (auto&& bootstrap_extension: bootstrap_extensions_) { + bootstrap_extension->serverInitialized(serverFactoryContext())); + } + // Instruct the listener manager to create the LDS provider if needed. This must be done later // because various items do not yet exist when the listener manager is created. if (bootstrap_.dynamic_resources().has_lds_config() || diff --git a/test/extensions/bootstrap/wasm/config_test.cc b/test/extensions/bootstrap/wasm/config_test.cc index a0b7274e53fd2..8bbb645a2abf7 100644 --- a/test/extensions/bootstrap/wasm/config_test.cc +++ b/test/extensions/bootstrap/wasm/config_test.cc @@ -52,7 +52,9 @@ class WasmFactoryTest : public testing::TestWithParam { EXPECT_CALL(context_, initManager()).WillRepeatedly(testing::ReturnRef(init_manager_)); EXPECT_CALL(context_, lifecycleNotifier()) .WillRepeatedly(testing::ReturnRef(lifecycle_notifier_)); - extension_ = factory->createBootstrapExtension(config, context_); + extension_ = factory->createBootstrapExtension(config, + context_.messageValidationContext().staticValidationVisitor()); + extension_->serverInitialized(context_); static_cast(extension_.get())->wasmService(); EXPECT_CALL(init_watcher_, ready()); init_manager_.initialize(init_watcher_); diff --git a/test/mocks/server/bootstrap_extension_factory.h b/test/mocks/server/bootstrap_extension_factory.h index f6421f7887880..30d074f58759a 100644 --- a/test/mocks/server/bootstrap_extension_factory.h +++ b/test/mocks/server/bootstrap_extension_factory.h @@ -13,7 +13,8 @@ class MockBootstrapExtensionFactory : public BootstrapExtensionFactory { ~MockBootstrapExtensionFactory() override; MOCK_METHOD(BootstrapExtensionPtr, createBootstrapExtension, - (const Protobuf::Message&, Configuration::ServerFactoryContext&), (override)); + (const Protobuf::Message&, + ProtobufMessage::ValidationVisitor& validation_visitor), (override)); MOCK_METHOD(ProtobufTypes::MessagePtr, createEmptyConfigProto, (), (override)); MOCK_METHOD(std::string, name, (), (const, override)); }; From 598c890447b5c760e09c5da08e9d646a9dd1dd96 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Thu, 17 Dec 2020 20:44:42 +0000 Subject: [PATCH 02/16] unit test; format fix Signed-off-by: Yuval Kohavi --- include/envoy/server/bootstrap_extension_config.h | 5 +++-- source/common/network/socket_interface.h | 1 + source/common/network/socket_interface_impl.cc | 2 +- source/common/network/socket_interface_impl.h | 2 +- source/extensions/bootstrap/wasm/config.cc | 7 ++++--- source/extensions/bootstrap/wasm/config.h | 7 ++----- source/server/server.cc | 9 +++++---- test/extensions/bootstrap/wasm/config_test.cc | 4 ++-- test/mocks/server/bootstrap_extension_factory.cc | 5 +++++ test/mocks/server/bootstrap_extension_factory.h | 14 ++++++++++++-- test/server/server_test.cc | 13 +++++++++---- 11 files changed, 45 insertions(+), 24 deletions(-) diff --git a/include/envoy/server/bootstrap_extension_config.h b/include/envoy/server/bootstrap_extension_config.h index 2c8abc1bd46b2..541fd51b8713b 100644 --- a/include/envoy/server/bootstrap_extension_config.h +++ b/include/envoy/server/bootstrap_extension_config.h @@ -39,8 +39,9 @@ class BootstrapExtensionFactory : public Config::TypedFactory { * @param config the custom configuration for this bootstrap extension type. * @param context general filter context through which persistent resources can be accessed. */ - virtual BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config, - ProtobufMessage::ValidationVisitor& validation_visitor) PURE; + virtual BootstrapExtensionPtr + createBootstrapExtension(const Protobuf::Message& config, + ProtobufMessage::ValidationVisitor& validation_visitor) PURE; std::string category() const override { return "envoy.bootstrap"; } }; diff --git a/source/common/network/socket_interface.h b/source/common/network/socket_interface.h index 49d517b0bdd34..af7d8aff91fef 100644 --- a/source/common/network/socket_interface.h +++ b/source/common/network/socket_interface.h @@ -19,6 +19,7 @@ class SocketInterfaceExtension : public Server::BootstrapExtension { SocketInterfaceExtension(SocketInterface& sock_interface) : sock_interface_(sock_interface) {} // Server::BootstrapExtension void serverInitialized(Server::Configuration::ServerFactoryContext&) override {} + protected: SocketInterface& sock_interface_; }; diff --git a/source/common/network/socket_interface_impl.cc b/source/common/network/socket_interface_impl.cc index 48f32278bcb21..c537ec754e547 100644 --- a/source/common/network/socket_interface_impl.cc +++ b/source/common/network/socket_interface_impl.cc @@ -91,7 +91,7 @@ bool SocketInterfaceImpl::ipFamilySupported(int domain) { Server::BootstrapExtensionPtr SocketInterfaceImpl::createBootstrapExtension(const Protobuf::Message&, - ProtobufMessage::ValidationVisitor&) { + ProtobufMessage::ValidationVisitor&) { return std::make_unique(*this); } diff --git a/source/common/network/socket_interface_impl.h b/source/common/network/socket_interface_impl.h index 7220b924deb86..4efabd06e24dd 100644 --- a/source/common/network/socket_interface_impl.h +++ b/source/common/network/socket_interface_impl.h @@ -19,7 +19,7 @@ class SocketInterfaceImpl : public SocketInterfaceBase { // Server::Configuration::BootstrapExtensionFactory Server::BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config, - ProtobufMessage::ValidationVisitor& validation_visitor) override; + ProtobufMessage::ValidationVisitor& validation_visitor) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; std::string name() const override { diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index 997a2d193f80d..412db681af723 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -36,8 +36,9 @@ void WasmServiceExtension::createWasm(Server::Configuration::ServerFactoryContex } if (config_.singleton()) { // Return a Wasm VM which will be stored as a singleton by the Server. - wasm_service_ = std::make_unique(plugin, Common::Wasm::getOrCreateThreadLocalPlugin( - base_wasm, plugin, context.dispatcher())); + wasm_service_ = std::make_unique( + plugin, + Common::Wasm::getOrCreateThreadLocalPlugin(base_wasm, plugin, context.dispatcher())); return; } // Per-thread WASM VM. @@ -63,7 +64,7 @@ void WasmServiceExtension::createWasm(Server::Configuration::ServerFactoryContex Server::BootstrapExtensionPtr WasmFactory::createBootstrapExtension(const Protobuf::Message& config, - ProtobufMessage::ValidationVisitor& validation_visitor) { + ProtobufMessage::ValidationVisitor& validation_visitor) { auto typed_config = MessageUtil::downcastAndValidate( config, validation_visitor); diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index 75fb19480cf4a..98b5aa33911b4 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -41,15 +41,13 @@ class WasmFactory : public Server::Configuration::BootstrapExtensionFactory { std::string name() const override { return "envoy.bootstrap.wasm"; } Server::BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config, - ProtobufMessage::ValidationVisitor& validation_visitor) override; + ProtobufMessage::ValidationVisitor& validation_visitor) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique(); } - }; -class WasmServiceExtension : public Server::BootstrapExtension, - Logger::Loggable { +class WasmServiceExtension : public Server::BootstrapExtension, Logger::Loggable { public: WasmServiceExtension(const envoy::extensions::wasm::v3::WasmService& config) : config_(config) {} WasmService& wasmService() { @@ -59,7 +57,6 @@ class WasmServiceExtension : public Server::BootstrapExtension, void serverInitialized(Server::Configuration::ServerFactoryContext& context) override; private: - void createWasm(Server::Configuration::ServerFactoryContext& context); envoy::extensions::wasm::v3::WasmService config_; diff --git a/source/server/server.cc b/source/server/server.cc index f70569481a2c6..c0323e3d117d5 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -139,7 +139,7 @@ InstanceImpl::~InstanceImpl() { ENVOY_LOG(debug, "destroyed listener manager"); } -Upstream::ClusterManager& InstanceImpl::clusterManager() { +Upstream::ClusterManager& InstanceImpl::clusterManager() { ASSERT(config_.clusterManager() != nullptr); return *config_.clusterManager(); } @@ -431,7 +431,8 @@ void InstanceImpl::initialize(const Options& options, auto config = Config::Utility::translateAnyToFactoryConfig( bootstrap_extension.typed_config(), messageValidationContext().staticValidationVisitor(), factory); - bootstrap_extensions_.push_back(factory.createBootstrapExtension(*config, messageValidationContext().staticValidationVisitor())); + bootstrap_extensions_.push_back(factory.createBootstrapExtension( + *config, messageValidationContext().staticValidationVisitor())); } // Register the fatal actions. @@ -536,8 +537,8 @@ void InstanceImpl::initialize(const Options& options, // cluster_manager_factory_ is available. config_.initialize(bootstrap_, *this, *cluster_manager_factory_); - for (auto&& bootstrap_extension: bootstrap_extensions_) { - bootstrap_extension->serverInitialized(serverFactoryContext())); + for (auto&& bootstrap_extension : bootstrap_extensions_) { + bootstrap_extension->serverInitialized(serverFactoryContext()); } // Instruct the listener manager to create the LDS provider if needed. This must be done later diff --git a/test/extensions/bootstrap/wasm/config_test.cc b/test/extensions/bootstrap/wasm/config_test.cc index 8bbb645a2abf7..edc2f0fae9856 100644 --- a/test/extensions/bootstrap/wasm/config_test.cc +++ b/test/extensions/bootstrap/wasm/config_test.cc @@ -52,8 +52,8 @@ class WasmFactoryTest : public testing::TestWithParam { EXPECT_CALL(context_, initManager()).WillRepeatedly(testing::ReturnRef(init_manager_)); EXPECT_CALL(context_, lifecycleNotifier()) .WillRepeatedly(testing::ReturnRef(lifecycle_notifier_)); - extension_ = factory->createBootstrapExtension(config, - context_.messageValidationContext().staticValidationVisitor()); + extension_ = factory->createBootstrapExtension( + config, context_.messageValidationContext().staticValidationVisitor()); extension_->serverInitialized(context_); static_cast(extension_.get())->wasmService(); EXPECT_CALL(init_watcher_, ready()); diff --git a/test/mocks/server/bootstrap_extension_factory.cc b/test/mocks/server/bootstrap_extension_factory.cc index 80984ea4093dc..1866e4ffb730a 100644 --- a/test/mocks/server/bootstrap_extension_factory.cc +++ b/test/mocks/server/bootstrap_extension_factory.cc @@ -2,6 +2,11 @@ namespace Envoy { namespace Server { + +MockBootstrapExtension::MockBootstrapExtension() = default; + +MockBootstrapExtension::~MockBootstrapExtension() = default; + namespace Configuration { MockBootstrapExtensionFactory::MockBootstrapExtensionFactory() = default; diff --git a/test/mocks/server/bootstrap_extension_factory.h b/test/mocks/server/bootstrap_extension_factory.h index 30d074f58759a..8637b9dd567d8 100644 --- a/test/mocks/server/bootstrap_extension_factory.h +++ b/test/mocks/server/bootstrap_extension_factory.h @@ -6,6 +6,15 @@ namespace Envoy { namespace Server { + +class MockBootstrapExtension : public BootstrapExtension { +public: + MockBootstrapExtension(); + ~MockBootstrapExtension() override; + + MOCK_METHOD(void, serverInitialized, (Configuration::ServerFactoryContext & context), (override)); +}; + namespace Configuration { class MockBootstrapExtensionFactory : public BootstrapExtensionFactory { public: @@ -13,11 +22,12 @@ class MockBootstrapExtensionFactory : public BootstrapExtensionFactory { ~MockBootstrapExtensionFactory() override; MOCK_METHOD(BootstrapExtensionPtr, createBootstrapExtension, - (const Protobuf::Message&, - ProtobufMessage::ValidationVisitor& validation_visitor), (override)); + (const Protobuf::Message&, ProtobufMessage::ValidationVisitor& validation_visitor), + (override)); MOCK_METHOD(ProtobufTypes::MessagePtr, createEmptyConfigProto, (), (override)); MOCK_METHOD(std::string, name, (), (const, override)); }; + } // namespace Configuration } // namespace Server } // namespace Envoy diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 162126c25b3c5..2e0e0ae33c531 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -1248,11 +1248,17 @@ TEST_P(ServerInstanceImplTest, WithBootstrapExtensions) { })); EXPECT_CALL(mock_factory, name()).WillRepeatedly(Return("envoy_test.bootstrap.foo")); EXPECT_CALL(mock_factory, createBootstrapExtension(_, _)) - .WillOnce(Invoke([](const Protobuf::Message& config, Configuration::ServerFactoryContext&) { + .WillOnce(Invoke([](const Protobuf::Message& config, ProtobufMessage::ValidationVisitor&) { const auto* proto = dynamic_cast(&config); EXPECT_NE(nullptr, proto); EXPECT_EQ(proto->a(), "foo"); - return std::make_unique(); + auto mock_extension = std::make_unique(); + EXPECT_CALL(*mock_extension, serverInitialized(_)) + .WillOnce(Invoke([](Configuration::ServerFactoryContext& ctx) { + // call to cluster manager, to make sure it is not nullptr. + ctx.clusterManager().clusters(); + })); + return mock_extension; })); Registry::InjectFactory registered_factory( @@ -1268,8 +1274,7 @@ TEST_P(ServerInstanceImplTest, WithBootstrapExtensionsThrowingError) { })); EXPECT_CALL(mock_factory, name()).WillRepeatedly(Return("envoy_test.bootstrap.foo")); EXPECT_CALL(mock_factory, createBootstrapExtension(_, _)) - .WillOnce(Invoke([](const Protobuf::Message&, - Configuration::ServerFactoryContext&) -> BootstrapExtensionPtr { + .WillOnce(Invoke([]() -> BootstrapExtensionPtr { throw EnvoyException("Unable to initiate mock_bootstrap_extension."); })); From 02e594acee1756ee42d12bd821f0d500df7cae76 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Thu, 17 Dec 2020 21:32:56 +0000 Subject: [PATCH 03/16] fix comments Signed-off-by: Yuval Kohavi --- include/envoy/server/bootstrap_extension_config.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/envoy/server/bootstrap_extension_config.h b/include/envoy/server/bootstrap_extension_config.h index 541fd51b8713b..a2e81646ae0a2 100644 --- a/include/envoy/server/bootstrap_extension_config.h +++ b/include/envoy/server/bootstrap_extension_config.h @@ -16,7 +16,10 @@ class BootstrapExtension { public: virtual ~BootstrapExtension() = default; - // Called when server is done initializing and we have the ServerFactoryConext available. + /** + * Called when server is done initializing and we have the ServerFactoryContext available. + * @param context general filter context through which persistent resources can be accessed. + */ virtual void serverInitialized(Configuration::ServerFactoryContext& context) PURE; }; @@ -37,7 +40,7 @@ class BootstrapExtensionFactory : public Config::TypedFactory { * implementation is unable to produce a factory with the provided parameters, it should throw an * EnvoyException. The returned pointer should never be nullptr. * @param config the custom configuration for this bootstrap extension type. - * @param context general filter context through which persistent resources can be accessed. + * @param validation_visitor message validation visitor instance. */ virtual BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config, From 13e92c51a648cd234d736e294ff383a0e0ed6f82 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Mon, 21 Dec 2020 17:11:01 +0000 Subject: [PATCH 04/16] onServerInitialized; add integration test Signed-off-by: Yuval Kohavi --- .../envoy/server/bootstrap_extension_config.h | 4 +- source/common/network/socket_interface.h | 2 +- source/extensions/bootstrap/wasm/config.cc | 3 +- source/extensions/bootstrap/wasm/config.h | 2 +- source/server/server.cc | 9 +- test/config/utility.cc | 6 ++ test/config/utility.h | 3 + test/extensions/bootstrap/wasm/BUILD | 16 ++++ test/extensions/bootstrap/wasm/config_test.cc | 2 +- .../extensions/bootstrap/wasm/test_data/BUILD | 5 ++ .../bootstrap/wasm/test_data/http_cpp.cc | 50 +++++++++++ .../bootstrap/wasm/wasm_integration_test.cc | 88 +++++++++++++++++++ .../server/bootstrap_extension_factory.h | 3 +- test/server/server_test.cc | 2 +- 14 files changed, 183 insertions(+), 12 deletions(-) create mode 100644 test/extensions/bootstrap/wasm/test_data/http_cpp.cc create mode 100644 test/extensions/bootstrap/wasm/wasm_integration_test.cc diff --git a/include/envoy/server/bootstrap_extension_config.h b/include/envoy/server/bootstrap_extension_config.h index a2e81646ae0a2..f6175e546aebf 100644 --- a/include/envoy/server/bootstrap_extension_config.h +++ b/include/envoy/server/bootstrap_extension_config.h @@ -18,9 +18,9 @@ class BootstrapExtension { /** * Called when server is done initializing and we have the ServerFactoryContext available. - * @param context general filter context through which persistent resources can be accessed. + * @param context is the context to use for the extension. */ - virtual void serverInitialized(Configuration::ServerFactoryContext& context) PURE; + virtual void onServerInitialized(Configuration::ServerFactoryContext& context) PURE; }; using BootstrapExtensionPtr = std::unique_ptr; diff --git a/source/common/network/socket_interface.h b/source/common/network/socket_interface.h index af7d8aff91fef..6fd76cdff8a32 100644 --- a/source/common/network/socket_interface.h +++ b/source/common/network/socket_interface.h @@ -18,7 +18,7 @@ class SocketInterfaceExtension : public Server::BootstrapExtension { public: SocketInterfaceExtension(SocketInterface& sock_interface) : sock_interface_(sock_interface) {} // Server::BootstrapExtension - void serverInitialized(Server::Configuration::ServerFactoryContext&) override {} + void onServerInitialized(Server::Configuration::ServerFactoryContext&) override {} protected: SocketInterface& sock_interface_; diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index 412db681af723..824e738875e86 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -14,7 +14,8 @@ namespace Extensions { namespace Bootstrap { namespace Wasm { -void WasmServiceExtension::serverInitialized(Server::Configuration::ServerFactoryContext& context) { +void WasmServiceExtension::onServerInitialized( + Server::Configuration::ServerFactoryContext& context) { createWasm(context); } diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index 98b5aa33911b4..796e8ba20c3eb 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -54,7 +54,7 @@ class WasmServiceExtension : public Server::BootstrapExtension, Logger::Loggable ASSERT(wasm_service_ != nullptr); return *wasm_service_; } - void serverInitialized(Server::Configuration::ServerFactoryContext& context) override; + void onServerInitialized(Server::Configuration::ServerFactoryContext& context) override; private: void createWasm(Server::Configuration::ServerFactoryContext& context); diff --git a/source/server/server.cc b/source/server/server.cc index c0323e3d117d5..487e9a38951be 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -537,10 +537,6 @@ void InstanceImpl::initialize(const Options& options, // cluster_manager_factory_ is available. config_.initialize(bootstrap_, *this, *cluster_manager_factory_); - for (auto&& bootstrap_extension : bootstrap_extensions_) { - bootstrap_extension->serverInitialized(serverFactoryContext()); - } - // Instruct the listener manager to create the LDS provider if needed. This must be done later // because various items do not yet exist when the listener manager is created. if (bootstrap_.dynamic_resources().has_lds_config() || @@ -573,6 +569,11 @@ void InstanceImpl::initialize(const Options& options, stat_flush_timer_->enableTimer(stats_config.flushInterval()); } + // Now that we are initialized, notify the bootstrap extensions. + for (auto&& bootstrap_extension : bootstrap_extensions_) { + bootstrap_extension->onServerInitialized(serverFactoryContext()); + } + // GuardDog (deadlock detection) object and thread setup before workers are // started and before our own run() loop runs. main_thread_guard_dog_ = std::make_unique( diff --git a/test/config/utility.cc b/test/config/utility.cc index e0e6012e520c4..fb07cc8adbdfd 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -1154,6 +1154,12 @@ void ConfigHelper::addListenerFilter(const std::string& filter_yaml) { } } +void ConfigHelper::addBootstrapExtension(const std::string& config) { + RELEASE_ASSERT(!finalized_, ""); + auto* extension = bootstrap_.add_bootstrap_extensions(); + TestUtility::loadFromYaml(config, *extension); +} + bool ConfigHelper::loadHttpConnectionManager( envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) { return loadFilter< diff --git a/test/config/utility.h b/test/config/utility.h index b51843eb3341d..9c934e270864e 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -199,6 +199,9 @@ class ConfigHelper { // Add a listener filter prior to existing filters. void addListenerFilter(const std::string& filter_yaml); + // Add a new bootstrap extension. + void addBootstrapExtension(const std::string& config); + // Sets the client codec to the specified type. void setClientCodec(envoy::extensions::filters::network::http_connection_manager::v3:: HttpConnectionManager::CodecType type); diff --git a/test/extensions/bootstrap/wasm/BUILD b/test/extensions/bootstrap/wasm/BUILD index b9c8282420c30..40be4228e31a6 100644 --- a/test/extensions/bootstrap/wasm/BUILD +++ b/test/extensions/bootstrap/wasm/BUILD @@ -45,6 +45,22 @@ envoy_extension_cc_test( ], ) +envoy_extension_cc_test( + name = "wasm_integration_test", + srcs = ["wasm_integration_test.cc"], + data = envoy_select_wasm([ + "//test/extensions/bootstrap/wasm/test_data:http_cpp.wasm", + ]), + extension_name = "envoy.bootstrap.wasm", + external_deps = ["abseil_optional"], + deps = [ + "//source/extensions/bootstrap/wasm:config", + "//source/extensions/common/wasm:wasm_lib", + "//test/extensions/common/wasm:wasm_runtime", + "//test/integration:http_protocol_integration_lib", + ], +) + envoy_extension_cc_test( name = "config_test", srcs = ["config_test.cc"], diff --git a/test/extensions/bootstrap/wasm/config_test.cc b/test/extensions/bootstrap/wasm/config_test.cc index edc2f0fae9856..8812e9b65f00a 100644 --- a/test/extensions/bootstrap/wasm/config_test.cc +++ b/test/extensions/bootstrap/wasm/config_test.cc @@ -54,7 +54,7 @@ class WasmFactoryTest : public testing::TestWithParam { .WillRepeatedly(testing::ReturnRef(lifecycle_notifier_)); extension_ = factory->createBootstrapExtension( config, context_.messageValidationContext().staticValidationVisitor()); - extension_->serverInitialized(context_); + extension_->onServerInitialized(context_); static_cast(extension_.get())->wasmService(); EXPECT_CALL(init_watcher_, ready()); init_manager_.initialize(init_watcher_); diff --git a/test/extensions/bootstrap/wasm/test_data/BUILD b/test/extensions/bootstrap/wasm/test_data/BUILD index d26678e60ee04..a8f2ec270f1a3 100644 --- a/test/extensions/bootstrap/wasm/test_data/BUILD +++ b/test/extensions/bootstrap/wasm/test_data/BUILD @@ -84,6 +84,11 @@ envoy_wasm_cc_binary( srcs = ["emscripten_cpp.cc"], ) +envoy_wasm_cc_binary( + name = "http_cpp.wasm", + srcs = ["http_cpp.cc"], +) + envoy_wasm_cc_binary( name = "logging_cpp.wasm", srcs = ["logging_cpp.cc"], diff --git a/test/extensions/bootstrap/wasm/test_data/http_cpp.cc b/test/extensions/bootstrap/wasm/test_data/http_cpp.cc new file mode 100644 index 0000000000000..afcad09d2feaf --- /dev/null +++ b/test/extensions/bootstrap/wasm/test_data/http_cpp.cc @@ -0,0 +1,50 @@ +// NOLINT(namespace-envoy) +#include + +#ifndef NULL_PLUGIN +#include "proxy_wasm_intrinsics.h" +#else +#include "include/proxy-wasm/null_plugin.h" +#endif + +template std::unique_ptr wrap_unique(T* ptr) { return std::unique_ptr(ptr); } + +START_WASM_PLUGIN(WasmHttpCpp) + +// Required Proxy-Wasm ABI version. +WASM_EXPORT(void, proxy_abi_version_0_1_0, ()) {} + +WASM_EXPORT(uint32_t, proxy_on_configure, (uint32_t, uint32_t)) { + proxy_set_tick_period_milliseconds(100); + return 1; +} + +WASM_EXPORT(void, proxy_on_tick, (uint32_t)) { + HeaderStringPairs headers; + headers.push_back(std::make_pair(":method", "GET")); + headers.push_back(std::make_pair(":path", "/")); + headers.push_back(std::make_pair(":authority", "example.com")); + HeaderStringPairs trailers; + uint32_t token; + WasmResult result = makeHttpCall("wasm_cluster", headers, "", trailers, 10000, &token); + // We have sent successfully, stop timer - we only want to send one request. + if (result == WasmResult::Ok) { + proxy_set_tick_period_milliseconds(0); + } +} + +WASM_EXPORT(void, proxy_on_http_call_response, (uint32_t context_id, uint32_t token, uint32_t headers, + uint32_t body_size, uint32_t trailers)) { + if (headers != 0) { + auto status = getHeaderMapValue(WasmHeaderMapType::HttpCallResponseHeaders, "status"); + if ("200" == status->view()) { + proxy_set_tick_period_milliseconds(0); + } else { + // Request failed - very possibly because of the integration test not being ready. + // Try again to prevent flakes. + proxy_set_tick_period_milliseconds(100); + } + } +} + +END_WASM_PLUGIN diff --git a/test/extensions/bootstrap/wasm/wasm_integration_test.cc b/test/extensions/bootstrap/wasm/wasm_integration_test.cc new file mode 100644 index 0000000000000..589909e1a415d --- /dev/null +++ b/test/extensions/bootstrap/wasm/wasm_integration_test.cc @@ -0,0 +1,88 @@ +#include "extensions/common/wasm/wasm.h" + +#include "test/integration/http_protocol_integration.h" + +#include "gtest/gtest.h" + +using testing::Eq; + +namespace Envoy { +namespace Extensions { +namespace Wasm { +namespace { + +class WasmIntegrationTest : public HttpProtocolIntegrationTest { +public: + void createUpstreams() override { + HttpIntegrationTest::createUpstreams(); + addFakeUpstream(GetParam().upstream_protocol); + } + + void cleanup() { + if (wasm_connection_ != nullptr) { + ASSERT_TRUE(wasm_connection_->close()); + ASSERT_TRUE(wasm_connection_->waitForDisconnect()); + } + cleanupUpstreamAndDownstream(); + } + void initialize() override { + auto httpwasm = TestEnvironment::substitute( + "{{ test_rundir }}/test/extensions/bootstrap/wasm/test_data/http_cpp.wasm"); + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto* wasm = bootstrap.mutable_static_resources()->add_clusters(); + wasm->MergeFrom(bootstrap.static_resources().clusters()[0]); + wasm->set_name("wasm_cluster"); + }); + + config_helper_.addBootstrapExtension(fmt::format(R"EOF( +name: envoy.filters.http.wasm +typed_config: + '@type': type.googleapis.com/envoy.extensions.wasm.v3.WasmService + singleton: true + config: + name: "singleton" + root_id: "singleton" + configuration: + '@type': type.googleapis.com/google.protobuf.StringValue + value: "" + vm_config: + vm_id: "my_vm_id" + runtime: "envoy.wasm.runtime.v8" + code: + local: + filename: {} + )EOF", + httpwasm)); + HttpIntegrationTest::initialize(); + } + + FakeHttpConnectionPtr wasm_connection_; + FakeStreamPtr wasm_request_; + IntegrationStreamDecoderPtr response_; +}; + +INSTANTIATE_TEST_SUITE_P(Protocols, WasmIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams()), + HttpProtocolIntegrationTest::protocolTestParamsToString); + +TEST_P(WasmIntegrationTest, FilterMakesCallInConfigureTime) { + initialize(); + ASSERT_TRUE(fake_upstreams_.back()->waitForHttpConnection(*dispatcher_, wasm_connection_)); + + // Expect the filter to send us an HTTP request + ASSERT_TRUE(wasm_connection_->waitForNewStream(*dispatcher_, wasm_request_)); + + ASSERT_TRUE(wasm_request_->waitForEndStream(*dispatcher_)); + + // Respond back to the filter. + Http::TestResponseHeaderMapImpl response_headers{ + {":status", "200"}, + }; + wasm_request_->encodeHeaders(response_headers, true); + cleanup(); +} + +} // namespace +} // namespace Wasm +} // namespace Extensions +} // namespace Envoy diff --git a/test/mocks/server/bootstrap_extension_factory.h b/test/mocks/server/bootstrap_extension_factory.h index 8637b9dd567d8..5190e911f4fc3 100644 --- a/test/mocks/server/bootstrap_extension_factory.h +++ b/test/mocks/server/bootstrap_extension_factory.h @@ -12,7 +12,8 @@ class MockBootstrapExtension : public BootstrapExtension { MockBootstrapExtension(); ~MockBootstrapExtension() override; - MOCK_METHOD(void, serverInitialized, (Configuration::ServerFactoryContext & context), (override)); + MOCK_METHOD(void, onServerInitialized, (Configuration::ServerFactoryContext & context), + (override)); }; namespace Configuration { diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 2e0e0ae33c531..e904ba9caac62 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -1253,7 +1253,7 @@ TEST_P(ServerInstanceImplTest, WithBootstrapExtensions) { EXPECT_NE(nullptr, proto); EXPECT_EQ(proto->a(), "foo"); auto mock_extension = std::make_unique(); - EXPECT_CALL(*mock_extension, serverInitialized(_)) + EXPECT_CALL(*mock_extension, onServerInitialized(_)) .WillOnce(Invoke([](Configuration::ServerFactoryContext& ctx) { // call to cluster manager, to make sure it is not nullptr. ctx.clusterManager().clusters(); From 87620b8ca3028bc480616b33081a66e27c4d95f7 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Mon, 21 Dec 2020 17:36:50 +0000 Subject: [PATCH 05/16] pr comment: bring back ServerFactoryContext to createBootstrapExtension Signed-off-by: Yuval Kohavi --- include/envoy/server/bootstrap_extension_config.h | 8 ++++---- source/common/network/socket_interface_impl.cc | 2 +- source/common/network/socket_interface_impl.h | 2 +- source/extensions/bootstrap/wasm/config.cc | 4 ++-- source/extensions/bootstrap/wasm/config.h | 2 +- source/server/server.cc | 4 ++-- test/extensions/bootstrap/wasm/config_test.cc | 3 +-- test/mocks/server/bootstrap_extension_factory.h | 3 +-- 8 files changed, 13 insertions(+), 15 deletions(-) diff --git a/include/envoy/server/bootstrap_extension_config.h b/include/envoy/server/bootstrap_extension_config.h index f6175e546aebf..1e3e398c83f0d 100644 --- a/include/envoy/server/bootstrap_extension_config.h +++ b/include/envoy/server/bootstrap_extension_config.h @@ -40,11 +40,11 @@ class BootstrapExtensionFactory : public Config::TypedFactory { * implementation is unable to produce a factory with the provided parameters, it should throw an * EnvoyException. The returned pointer should never be nullptr. * @param config the custom configuration for this bootstrap extension type. - * @param validation_visitor message validation visitor instance. + * @param context is the context to use for the extension. Note that the clusterManager is not + * yet initialized at this point and **must not** be used. */ - virtual BootstrapExtensionPtr - createBootstrapExtension(const Protobuf::Message& config, - ProtobufMessage::ValidationVisitor& validation_visitor) PURE; + virtual BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config, + ServerFactoryContext& context) PURE; std::string category() const override { return "envoy.bootstrap"; } }; diff --git a/source/common/network/socket_interface_impl.cc b/source/common/network/socket_interface_impl.cc index c537ec754e547..8a3802e36769e 100644 --- a/source/common/network/socket_interface_impl.cc +++ b/source/common/network/socket_interface_impl.cc @@ -91,7 +91,7 @@ bool SocketInterfaceImpl::ipFamilySupported(int domain) { Server::BootstrapExtensionPtr SocketInterfaceImpl::createBootstrapExtension(const Protobuf::Message&, - ProtobufMessage::ValidationVisitor&) { + Server::Configuration::ServerFactoryContext&) { return std::make_unique(*this); } diff --git a/source/common/network/socket_interface_impl.h b/source/common/network/socket_interface_impl.h index 4efabd06e24dd..ecc14e2e551b1 100644 --- a/source/common/network/socket_interface_impl.h +++ b/source/common/network/socket_interface_impl.h @@ -19,7 +19,7 @@ class SocketInterfaceImpl : public SocketInterfaceBase { // Server::Configuration::BootstrapExtensionFactory Server::BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config, - ProtobufMessage::ValidationVisitor& validation_visitor) override; + Server::Configuration::ServerFactoryContext& context) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override; std::string name() const override { diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index 824e738875e86..1626ce4528963 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -65,10 +65,10 @@ void WasmServiceExtension::createWasm(Server::Configuration::ServerFactoryContex Server::BootstrapExtensionPtr WasmFactory::createBootstrapExtension(const Protobuf::Message& config, - ProtobufMessage::ValidationVisitor& validation_visitor) { + Server::Configuration::ServerFactoryContext& context) { auto typed_config = MessageUtil::downcastAndValidate( - config, validation_visitor); + config, context.messageValidationContext().staticValidationVisitor()); return std::make_unique(typed_config); } diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index 796e8ba20c3eb..de251600daf02 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -41,7 +41,7 @@ class WasmFactory : public Server::Configuration::BootstrapExtensionFactory { std::string name() const override { return "envoy.bootstrap.wasm"; } Server::BootstrapExtensionPtr createBootstrapExtension(const Protobuf::Message& config, - ProtobufMessage::ValidationVisitor& validation_visitor) override; + Server::Configuration::ServerFactoryContext& context) override; ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique(); } diff --git a/source/server/server.cc b/source/server/server.cc index 487e9a38951be..8a895b83dcb98 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -431,8 +431,8 @@ void InstanceImpl::initialize(const Options& options, auto config = Config::Utility::translateAnyToFactoryConfig( bootstrap_extension.typed_config(), messageValidationContext().staticValidationVisitor(), factory); - bootstrap_extensions_.push_back(factory.createBootstrapExtension( - *config, messageValidationContext().staticValidationVisitor())); + bootstrap_extensions_.push_back( + factory.createBootstrapExtension(*config, serverFactoryContext())); } // Register the fatal actions. diff --git a/test/extensions/bootstrap/wasm/config_test.cc b/test/extensions/bootstrap/wasm/config_test.cc index 8812e9b65f00a..3ec94b47c90e4 100644 --- a/test/extensions/bootstrap/wasm/config_test.cc +++ b/test/extensions/bootstrap/wasm/config_test.cc @@ -52,8 +52,7 @@ class WasmFactoryTest : public testing::TestWithParam { EXPECT_CALL(context_, initManager()).WillRepeatedly(testing::ReturnRef(init_manager_)); EXPECT_CALL(context_, lifecycleNotifier()) .WillRepeatedly(testing::ReturnRef(lifecycle_notifier_)); - extension_ = factory->createBootstrapExtension( - config, context_.messageValidationContext().staticValidationVisitor()); + extension_ = factory->createBootstrapExtension(config, context_); extension_->onServerInitialized(context_); static_cast(extension_.get())->wasmService(); EXPECT_CALL(init_watcher_, ready()); diff --git a/test/mocks/server/bootstrap_extension_factory.h b/test/mocks/server/bootstrap_extension_factory.h index 5190e911f4fc3..5ab0c76b2aced 100644 --- a/test/mocks/server/bootstrap_extension_factory.h +++ b/test/mocks/server/bootstrap_extension_factory.h @@ -23,8 +23,7 @@ class MockBootstrapExtensionFactory : public BootstrapExtensionFactory { ~MockBootstrapExtensionFactory() override; MOCK_METHOD(BootstrapExtensionPtr, createBootstrapExtension, - (const Protobuf::Message&, ProtobufMessage::ValidationVisitor& validation_visitor), - (override)); + (const Protobuf::Message&, Configuration::ServerFactoryContext& context), (override)); MOCK_METHOD(ProtobufTypes::MessagePtr, createEmptyConfigProto, (), (override)); MOCK_METHOD(std::string, name, (), (const, override)); }; From dbe9055ede921d1ba4ef2543f6cc883e611990be Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Mon, 21 Dec 2020 17:57:16 +0000 Subject: [PATCH 06/16] minor: test compile fix Signed-off-by: Yuval Kohavi --- test/mocks/server/bootstrap_extension_factory.h | 3 +-- test/server/server_test.cc | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/mocks/server/bootstrap_extension_factory.h b/test/mocks/server/bootstrap_extension_factory.h index 5ab0c76b2aced..5c00b9385ae69 100644 --- a/test/mocks/server/bootstrap_extension_factory.h +++ b/test/mocks/server/bootstrap_extension_factory.h @@ -23,11 +23,10 @@ class MockBootstrapExtensionFactory : public BootstrapExtensionFactory { ~MockBootstrapExtensionFactory() override; MOCK_METHOD(BootstrapExtensionPtr, createBootstrapExtension, - (const Protobuf::Message&, Configuration::ServerFactoryContext& context), (override)); + (const Protobuf::Message&, Configuration::ServerFactoryContext&), (override)); MOCK_METHOD(ProtobufTypes::MessagePtr, createEmptyConfigProto, (), (override)); MOCK_METHOD(std::string, name, (), (const, override)); }; - } // namespace Configuration } // namespace Server } // namespace Envoy diff --git a/test/server/server_test.cc b/test/server/server_test.cc index e904ba9caac62..64107c93d649a 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -1248,7 +1248,7 @@ TEST_P(ServerInstanceImplTest, WithBootstrapExtensions) { })); EXPECT_CALL(mock_factory, name()).WillRepeatedly(Return("envoy_test.bootstrap.foo")); EXPECT_CALL(mock_factory, createBootstrapExtension(_, _)) - .WillOnce(Invoke([](const Protobuf::Message& config, ProtobufMessage::ValidationVisitor&) { + .WillOnce(Invoke([](const Protobuf::Message& config, Configuration::ServerFactoryContext&) { const auto* proto = dynamic_cast(&config); EXPECT_NE(nullptr, proto); EXPECT_EQ(proto->a(), "foo"); @@ -1274,7 +1274,8 @@ TEST_P(ServerInstanceImplTest, WithBootstrapExtensionsThrowingError) { })); EXPECT_CALL(mock_factory, name()).WillRepeatedly(Return("envoy_test.bootstrap.foo")); EXPECT_CALL(mock_factory, createBootstrapExtension(_, _)) - .WillOnce(Invoke([]() -> BootstrapExtensionPtr { + .WillOnce(Invoke([](const Protobuf::Message&, + Configuration::ServerFactoryContext&) -> BootstrapExtensionPtr { throw EnvoyException("Unable to initiate mock_bootstrap_extension."); })); From 919e67fd8fdf495bdb779aa3176bdb41589b4557 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Mon, 21 Dec 2020 20:09:01 +0000 Subject: [PATCH 07/16] tidy Signed-off-by: Yuval Kohavi --- test/extensions/bootstrap/wasm/BUILD | 1 - test/extensions/bootstrap/wasm/wasm_integration_test.cc | 2 -- 2 files changed, 3 deletions(-) diff --git a/test/extensions/bootstrap/wasm/BUILD b/test/extensions/bootstrap/wasm/BUILD index 40be4228e31a6..eb3d63864f825 100644 --- a/test/extensions/bootstrap/wasm/BUILD +++ b/test/extensions/bootstrap/wasm/BUILD @@ -52,7 +52,6 @@ envoy_extension_cc_test( "//test/extensions/bootstrap/wasm/test_data:http_cpp.wasm", ]), extension_name = "envoy.bootstrap.wasm", - external_deps = ["abseil_optional"], deps = [ "//source/extensions/bootstrap/wasm:config", "//source/extensions/common/wasm:wasm_lib", diff --git a/test/extensions/bootstrap/wasm/wasm_integration_test.cc b/test/extensions/bootstrap/wasm/wasm_integration_test.cc index 589909e1a415d..cc27f18740428 100644 --- a/test/extensions/bootstrap/wasm/wasm_integration_test.cc +++ b/test/extensions/bootstrap/wasm/wasm_integration_test.cc @@ -4,8 +4,6 @@ #include "gtest/gtest.h" -using testing::Eq; - namespace Envoy { namespace Extensions { namespace Wasm { From 039041a588c59bb90d2cdc718e8cab2532c0b689 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Tue, 22 Dec 2020 21:58:33 +0000 Subject: [PATCH 08/16] parameterize over runtimes Signed-off-by: Yuval Kohavi --- .../bootstrap/wasm/test_data/http_cpp.cc | 15 +++++---------- .../bootstrap/wasm/wasm_integration_test.cc | 18 +++++++++++------- test/extensions/common/wasm/wasm_runtime.cc | 4 ++++ test/extensions/common/wasm/wasm_runtime.h | 2 ++ 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/test/extensions/bootstrap/wasm/test_data/http_cpp.cc b/test/extensions/bootstrap/wasm/test_data/http_cpp.cc index afcad09d2feaf..f9b96b814fa6b 100644 --- a/test/extensions/bootstrap/wasm/test_data/http_cpp.cc +++ b/test/extensions/bootstrap/wasm/test_data/http_cpp.cc @@ -1,11 +1,7 @@ // NOLINT(namespace-envoy) #include -#ifndef NULL_PLUGIN #include "proxy_wasm_intrinsics.h" -#else -#include "include/proxy-wasm/null_plugin.h" -#endif template std::unique_ptr wrap_unique(T* ptr) { return std::unique_ptr(ptr); } @@ -33,18 +29,17 @@ WASM_EXPORT(void, proxy_on_tick, (uint32_t)) { } } -WASM_EXPORT(void, proxy_on_http_call_response, (uint32_t context_id, uint32_t token, uint32_t headers, - uint32_t body_size, uint32_t trailers)) { +WASM_EXPORT(void, proxy_on_http_call_response, (uint32_t, uint32_t, uint32_t headers, uint32_t, uint32_t)) { if (headers != 0) { auto status = getHeaderMapValue(WasmHeaderMapType::HttpCallResponseHeaders, "status"); if ("200" == status->view()) { proxy_set_tick_period_milliseconds(0); - } else { - // Request failed - very possibly because of the integration test not being ready. - // Try again to prevent flakes. - proxy_set_tick_period_milliseconds(100); + return; } } + // Request failed - very possibly because of the integration test not being ready. + // Try again to prevent flakes. + proxy_set_tick_period_milliseconds(100); } END_WASM_PLUGIN diff --git a/test/extensions/bootstrap/wasm/wasm_integration_test.cc b/test/extensions/bootstrap/wasm/wasm_integration_test.cc index cc27f18740428..82d8ea6d752b3 100644 --- a/test/extensions/bootstrap/wasm/wasm_integration_test.cc +++ b/test/extensions/bootstrap/wasm/wasm_integration_test.cc @@ -1,5 +1,6 @@ #include "extensions/common/wasm/wasm.h" +#include "test/extensions/common/wasm/wasm_runtime.h" #include "test/integration/http_protocol_integration.h" #include "gtest/gtest.h" @@ -9,11 +10,14 @@ namespace Extensions { namespace Wasm { namespace { -class WasmIntegrationTest : public HttpProtocolIntegrationTest { +class WasmIntegrationTest : public HttpIntegrationTest, public testing::TestWithParam { public: + WasmIntegrationTest() + : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, Network::Address::IpVersion::v4) {} + void createUpstreams() override { HttpIntegrationTest::createUpstreams(); - addFakeUpstream(GetParam().upstream_protocol); + addFakeUpstream(FakeHttpConnection::Type::HTTP1); } void cleanup() { @@ -45,12 +49,12 @@ name: envoy.filters.http.wasm value: "" vm_config: vm_id: "my_vm_id" - runtime: "envoy.wasm.runtime.v8" + runtime: "envoy.wasm.runtime.{}" code: local: filename: {} )EOF", - httpwasm)); + GetParam(), httpwasm)); HttpIntegrationTest::initialize(); } @@ -59,9 +63,9 @@ name: envoy.filters.http.wasm IntegrationStreamDecoderPtr response_; }; -INSTANTIATE_TEST_SUITE_P(Protocols, WasmIntegrationTest, - testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams()), - HttpProtocolIntegrationTest::protocolTestParamsToString); +INSTANTIATE_TEST_SUITE_P(Runtimes, WasmIntegrationTest, + Envoy::Extensions::Common::Wasm::sandbox_runtime_values, + Envoy::Extensions::Common::Wasm::wasmTestParamsToString); TEST_P(WasmIntegrationTest, FilterMakesCallInConfigureTime) { initialize(); diff --git a/test/extensions/common/wasm/wasm_runtime.cc b/test/extensions/common/wasm/wasm_runtime.cc index a8451c70df642..e39288e12d0bb 100644 --- a/test/extensions/common/wasm/wasm_runtime.cc +++ b/test/extensions/common/wasm/wasm_runtime.cc @@ -35,6 +35,10 @@ std::vector> runtimesAndLanguages() { return values; } +std::string wasmTestParamsToString(const ::testing::TestParamInfo& p) { + return p.param; +} + } // namespace Wasm } // namespace Common } // namespace Extensions diff --git a/test/extensions/common/wasm/wasm_runtime.h b/test/extensions/common/wasm/wasm_runtime.h index ef248d85310b2..5c1d73fbd08cf 100644 --- a/test/extensions/common/wasm/wasm_runtime.h +++ b/test/extensions/common/wasm/wasm_runtime.h @@ -20,6 +20,8 @@ inline auto runtime_values = testing::ValuesIn(runtimes()); inline auto sandbox_runtime_values = testing::ValuesIn(sandboxRuntimes()); inline auto runtime_and_language_values = testing::ValuesIn(runtimesAndLanguages()); +std::string wasmTestParamsToString(const ::testing::TestParamInfo& p); + } // namespace Wasm } // namespace Common } // namespace Extensions From 37e53acfda63021d3b66f8ab87122f9d1d703aa6 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Wed, 23 Dec 2020 13:25:21 +0000 Subject: [PATCH 09/16] allow uninstantiated Signed-off-by: Yuval Kohavi --- test/extensions/bootstrap/wasm/wasm_integration_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/extensions/bootstrap/wasm/wasm_integration_test.cc b/test/extensions/bootstrap/wasm/wasm_integration_test.cc index 82d8ea6d752b3..e27079a287b44 100644 --- a/test/extensions/bootstrap/wasm/wasm_integration_test.cc +++ b/test/extensions/bootstrap/wasm/wasm_integration_test.cc @@ -66,6 +66,7 @@ name: envoy.filters.http.wasm INSTANTIATE_TEST_SUITE_P(Runtimes, WasmIntegrationTest, Envoy::Extensions::Common::Wasm::sandbox_runtime_values, Envoy::Extensions::Common::Wasm::wasmTestParamsToString); +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WasmIntegrationTest); TEST_P(WasmIntegrationTest, FilterMakesCallInConfigureTime) { initialize(); From d6264ad3de2e2fee5c86f3fb08fa3669ef2a15bb Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Thu, 14 Jan 2021 13:39:57 -0500 Subject: [PATCH 10/16] remove context from onServerInitialized Signed-off-by: Yuval Kohavi --- source/common/network/socket_interface.h | 2 +- source/extensions/bootstrap/wasm/config.cc | 7 +++---- source/extensions/bootstrap/wasm/config.h | 5 +++-- source/server/server.cc | 2 +- test/extensions/bootstrap/wasm/config_test.cc | 2 +- test/mocks/server/bootstrap_extension_factory.h | 3 +-- test/server/server_test.cc | 2 +- 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/source/common/network/socket_interface.h b/source/common/network/socket_interface.h index 6fd76cdff8a32..89ae1d9770ded 100644 --- a/source/common/network/socket_interface.h +++ b/source/common/network/socket_interface.h @@ -18,7 +18,7 @@ class SocketInterfaceExtension : public Server::BootstrapExtension { public: SocketInterfaceExtension(SocketInterface& sock_interface) : sock_interface_(sock_interface) {} // Server::BootstrapExtension - void onServerInitialized(Server::Configuration::ServerFactoryContext&) override {} + void onServerInitialized() override {} protected: SocketInterface& sock_interface_; diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index 1626ce4528963..2e4621cbaecf8 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -14,9 +14,8 @@ namespace Extensions { namespace Bootstrap { namespace Wasm { -void WasmServiceExtension::onServerInitialized( - Server::Configuration::ServerFactoryContext& context) { - createWasm(context); +void WasmServiceExtension::onServerInitialized() { + createWasm(context_); } void WasmServiceExtension::createWasm(Server::Configuration::ServerFactoryContext& context) { @@ -70,7 +69,7 @@ WasmFactory::createBootstrapExtension(const Protobuf::Message& config, MessageUtil::downcastAndValidate( config, context.messageValidationContext().staticValidationVisitor()); - return std::make_unique(typed_config); + return std::make_unique(typed_config, context); } // /** diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index de251600daf02..9f0626f8b949a 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -49,17 +49,18 @@ class WasmFactory : public Server::Configuration::BootstrapExtensionFactory { class WasmServiceExtension : public Server::BootstrapExtension, Logger::Loggable { public: - WasmServiceExtension(const envoy::extensions::wasm::v3::WasmService& config) : config_(config) {} + WasmServiceExtension(const envoy::extensions::wasm::v3::WasmService& config, Server::Configuration::ServerFactoryContext& context) : config_(config), context_(context) {} WasmService& wasmService() { ASSERT(wasm_service_ != nullptr); return *wasm_service_; } - void onServerInitialized(Server::Configuration::ServerFactoryContext& context) override; + void onServerInitialized() override; private: void createWasm(Server::Configuration::ServerFactoryContext& context); envoy::extensions::wasm::v3::WasmService config_; + Server::Configuration::ServerFactoryContext& context_; WasmServicePtr wasm_service_; Config::DataSource::RemoteAsyncDataProviderPtr remote_data_provider_; }; diff --git a/source/server/server.cc b/source/server/server.cc index f09cb693c2762..8ae316704036e 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -571,7 +571,7 @@ void InstanceImpl::initialize(const Options& options, // Now that we are initialized, notify the bootstrap extensions. for (auto&& bootstrap_extension : bootstrap_extensions_) { - bootstrap_extension->onServerInitialized(serverFactoryContext()); + bootstrap_extension->onServerInitialized(); } // GuardDog (deadlock detection) object and thread setup before workers are diff --git a/test/extensions/bootstrap/wasm/config_test.cc b/test/extensions/bootstrap/wasm/config_test.cc index 3ec94b47c90e4..560a60d9cd56c 100644 --- a/test/extensions/bootstrap/wasm/config_test.cc +++ b/test/extensions/bootstrap/wasm/config_test.cc @@ -53,7 +53,7 @@ class WasmFactoryTest : public testing::TestWithParam { EXPECT_CALL(context_, lifecycleNotifier()) .WillRepeatedly(testing::ReturnRef(lifecycle_notifier_)); extension_ = factory->createBootstrapExtension(config, context_); - extension_->onServerInitialized(context_); + extension_->onServerInitialized(); static_cast(extension_.get())->wasmService(); EXPECT_CALL(init_watcher_, ready()); init_manager_.initialize(init_watcher_); diff --git a/test/mocks/server/bootstrap_extension_factory.h b/test/mocks/server/bootstrap_extension_factory.h index 5c00b9385ae69..9ae9940cec8c4 100644 --- a/test/mocks/server/bootstrap_extension_factory.h +++ b/test/mocks/server/bootstrap_extension_factory.h @@ -12,8 +12,7 @@ class MockBootstrapExtension : public BootstrapExtension { MockBootstrapExtension(); ~MockBootstrapExtension() override; - MOCK_METHOD(void, onServerInitialized, (Configuration::ServerFactoryContext & context), - (override)); + MOCK_METHOD(void, onServerInitialized, (), (override)); }; namespace Configuration { diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 6d9461a703414..6be2588fc0dd0 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -1326,7 +1326,7 @@ TEST_P(ServerInstanceImplTest, WithBootstrapExtensions) { EXPECT_NE(nullptr, proto); EXPECT_EQ(proto->a(), "foo"); auto mock_extension = std::make_unique(); - EXPECT_CALL(*mock_extension, onServerInitialized(_)) + EXPECT_CALL(*mock_extension, onServerInitialized()) .WillOnce(Invoke([](Configuration::ServerFactoryContext& ctx) { // call to cluster manager, to make sure it is not nullptr. ctx.clusterManager().clusters(); From d181b020061c90e92d45ebb3b27c42eb0f300ed3 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Thu, 14 Jan 2021 18:52:31 +0000 Subject: [PATCH 11/16] format fix Signed-off-by: Yuval Kohavi --- source/extensions/bootstrap/wasm/config.cc | 4 +--- source/extensions/bootstrap/wasm/config.h | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source/extensions/bootstrap/wasm/config.cc b/source/extensions/bootstrap/wasm/config.cc index 2e4621cbaecf8..545a761391d22 100644 --- a/source/extensions/bootstrap/wasm/config.cc +++ b/source/extensions/bootstrap/wasm/config.cc @@ -14,9 +14,7 @@ namespace Extensions { namespace Bootstrap { namespace Wasm { -void WasmServiceExtension::onServerInitialized() { - createWasm(context_); -} +void WasmServiceExtension::onServerInitialized() { createWasm(context_); } void WasmServiceExtension::createWasm(Server::Configuration::ServerFactoryContext& context) { auto plugin = std::make_shared( diff --git a/source/extensions/bootstrap/wasm/config.h b/source/extensions/bootstrap/wasm/config.h index 9f0626f8b949a..9d09a7905b2d4 100644 --- a/source/extensions/bootstrap/wasm/config.h +++ b/source/extensions/bootstrap/wasm/config.h @@ -49,7 +49,9 @@ class WasmFactory : public Server::Configuration::BootstrapExtensionFactory { class WasmServiceExtension : public Server::BootstrapExtension, Logger::Loggable { public: - WasmServiceExtension(const envoy::extensions::wasm::v3::WasmService& config, Server::Configuration::ServerFactoryContext& context) : config_(config), context_(context) {} + WasmServiceExtension(const envoy::extensions::wasm::v3::WasmService& config, + Server::Configuration::ServerFactoryContext& context) + : config_(config), context_(context) {} WasmService& wasmService() { ASSERT(wasm_service_ != nullptr); return *wasm_service_; From 2b39bbc0896de15f01556701197b8a8a358b0a20 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Thu, 14 Jan 2021 19:39:28 +0000 Subject: [PATCH 12/16] remove arg from interface Signed-off-by: Yuval Kohavi --- include/envoy/server/bootstrap_extension_config.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/envoy/server/bootstrap_extension_config.h b/include/envoy/server/bootstrap_extension_config.h index 1e3e398c83f0d..f674540d19fb5 100644 --- a/include/envoy/server/bootstrap_extension_config.h +++ b/include/envoy/server/bootstrap_extension_config.h @@ -17,10 +17,9 @@ class BootstrapExtension { virtual ~BootstrapExtension() = default; /** - * Called when server is done initializing and we have the ServerFactoryContext available. - * @param context is the context to use for the extension. + * Called when server is done initializing and we have the ServerFactoryContext fully initialized. */ - virtual void onServerInitialized(Configuration::ServerFactoryContext& context) PURE; + virtual void onServerInitialized() PURE; }; using BootstrapExtensionPtr = std::unique_ptr; From b95ecaf34334307e38b168c71c053298d1a92c97 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Thu, 14 Jan 2021 19:45:41 +0000 Subject: [PATCH 13/16] add header to integration test Signed-off-by: Yuval Kohavi --- test/extensions/bootstrap/wasm/test_data/http_cpp.cc | 1 + test/extensions/bootstrap/wasm/wasm_integration_test.cc | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/extensions/bootstrap/wasm/test_data/http_cpp.cc b/test/extensions/bootstrap/wasm/test_data/http_cpp.cc index f9b96b814fa6b..7b70f7b7d4e00 100644 --- a/test/extensions/bootstrap/wasm/test_data/http_cpp.cc +++ b/test/extensions/bootstrap/wasm/test_data/http_cpp.cc @@ -20,6 +20,7 @@ WASM_EXPORT(void, proxy_on_tick, (uint32_t)) { headers.push_back(std::make_pair(":method", "GET")); headers.push_back(std::make_pair(":path", "/")); headers.push_back(std::make_pair(":authority", "example.com")); + headers.push_back(std::make_pair("x-test", "test")); HeaderStringPairs trailers; uint32_t token; WasmResult result = makeHttpCall("wasm_cluster", headers, "", trailers, 10000, &token); diff --git a/test/extensions/bootstrap/wasm/wasm_integration_test.cc b/test/extensions/bootstrap/wasm/wasm_integration_test.cc index e27079a287b44..6d022664a85b9 100644 --- a/test/extensions/bootstrap/wasm/wasm_integration_test.cc +++ b/test/extensions/bootstrap/wasm/wasm_integration_test.cc @@ -74,9 +74,13 @@ TEST_P(WasmIntegrationTest, FilterMakesCallInConfigureTime) { // Expect the filter to send us an HTTP request ASSERT_TRUE(wasm_connection_->waitForNewStream(*dispatcher_, wasm_request_)); - ASSERT_TRUE(wasm_request_->waitForEndStream(*dispatcher_)); + EXPECT_EQ("test", wasm_request_->headers() + .get(Envoy::Http::LowerCaseString("x-test"))[0] + ->value() + .getStringView()); + // Respond back to the filter. Http::TestResponseHeaderMapImpl response_headers{ {":status", "200"}, From 2354ea28f2d36764304af4fc5ac241e562e965f2 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Thu, 14 Jan 2021 17:40:31 -0500 Subject: [PATCH 14/16] compile fix Signed-off-by: Yuval Kohavi --- test/server/server_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 6be2588fc0dd0..16df13298d131 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -1327,7 +1327,7 @@ TEST_P(ServerInstanceImplTest, WithBootstrapExtensions) { EXPECT_EQ(proto->a(), "foo"); auto mock_extension = std::make_unique(); EXPECT_CALL(*mock_extension, onServerInitialized()) - .WillOnce(Invoke([](Configuration::ServerFactoryContext& ctx) { + .WillOnce(Invoke([]() { // call to cluster manager, to make sure it is not nullptr. ctx.clusterManager().clusters(); })); From b96f11674f5137fde076a67dfe5a9cc0ac741c61 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Fri, 15 Jan 2021 14:44:01 +0000 Subject: [PATCH 15/16] format fix Signed-off-by: Yuval Kohavi --- test/server/server_test.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 16df13298d131..c62d74f5ae8b6 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -1326,11 +1326,10 @@ TEST_P(ServerInstanceImplTest, WithBootstrapExtensions) { EXPECT_NE(nullptr, proto); EXPECT_EQ(proto->a(), "foo"); auto mock_extension = std::make_unique(); - EXPECT_CALL(*mock_extension, onServerInitialized()) - .WillOnce(Invoke([]() { - // call to cluster manager, to make sure it is not nullptr. - ctx.clusterManager().clusters(); - })); + EXPECT_CALL(*mock_extension, onServerInitialized()).WillOnce(Invoke([]() { + // call to cluster manager, to make sure it is not nullptr. + ctx.clusterManager().clusters(); + })); return mock_extension; })); From fddcd702146b9d62fa1991ff9b316151feda4799 Mon Sep 17 00:00:00 2001 From: Yuval Kohavi Date: Fri, 15 Jan 2021 16:39:18 +0000 Subject: [PATCH 16/16] minor fix Signed-off-by: Yuval Kohavi --- test/server/server_test.cc | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/test/server/server_test.cc b/test/server/server_test.cc index c62d74f5ae8b6..4f275a9c7629d 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -1320,18 +1320,20 @@ TEST_P(ServerInstanceImplTest, WithBootstrapExtensions) { return std::make_unique(); })); EXPECT_CALL(mock_factory, name()).WillRepeatedly(Return("envoy_test.bootstrap.foo")); + EXPECT_CALL(mock_factory, createBootstrapExtension(_, _)) - .WillOnce(Invoke([](const Protobuf::Message& config, Configuration::ServerFactoryContext&) { - const auto* proto = dynamic_cast(&config); - EXPECT_NE(nullptr, proto); - EXPECT_EQ(proto->a(), "foo"); - auto mock_extension = std::make_unique(); - EXPECT_CALL(*mock_extension, onServerInitialized()).WillOnce(Invoke([]() { - // call to cluster manager, to make sure it is not nullptr. - ctx.clusterManager().clusters(); - })); - return mock_extension; - })); + .WillOnce( + Invoke([](const Protobuf::Message& config, Configuration::ServerFactoryContext& ctx) { + const auto* proto = dynamic_cast(&config); + EXPECT_NE(nullptr, proto); + EXPECT_EQ(proto->a(), "foo"); + auto mock_extension = std::make_unique(); + EXPECT_CALL(*mock_extension, onServerInitialized()).WillOnce(Invoke([&ctx]() { + // call to cluster manager, to make sure it is not nullptr. + ctx.clusterManager().clusters(); + })); + return mock_extension; + })); Registry::InjectFactory registered_factory( mock_factory);