bootstrap-extensions: fix a crash on http callout

include/envoy/server/bootstrap_extension_config.h

@@ -15,6 +15,12 @@ namespace Server {
 class BootstrapExtension {
 public:
   virtual ~BootstrapExtension() = default;
+
+  /**
+   * 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;
 };
 
 using BootstrapExtensionPtr = std::unique_ptr<BootstrapExtension>;
@@ -34,10 +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 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,
-                                                         ServerFactoryContext& context) PURE;
+  virtual BootstrapExtensionPtr
+  createBootstrapExtension(const Protobuf::Message& config,
+                           ProtobufMessage::ValidationVisitor& validation_visitor) PURE;
 
   std::string category() const override { return "envoy.bootstrap"; }
 };

source/common/network/socket_interface.h

@@ -17,6 +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_;

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<SocketInterfaceExtension>(*this);
 }
 

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";

source/extensions/bootstrap/wasm/config.cc

@@ -14,19 +14,18 @@ namespace Extensions {
 namespace Bootstrap {
 namespace Wasm {
 
-static const std::string INLINE_STRING = "<inline>";
+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<Common::Wasm::Plugin>(
-      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,11 @@ 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<WasmService>(plugin, Common::Wasm::getOrCreateThreadLocalPlugin(
-                                                   base_wasm, plugin, context.dispatcher())));
+      wasm_service_ = std::make_unique<WasmService>(
+          plugin,
+          Common::Wasm::getOrCreateThreadLocalPlugin(base_wasm, plugin, context.dispatcher()));
       return;
     }
     // Per-thread WASM VM.
@@ -48,11 +48,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<WasmService>(plugin, std::move(tls_slot)));
+    wasm_service_ = std::make_unique<WasmService>(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 +64,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<const envoy::extensions::wasm::v3::WasmService&>(
-          config, context.messageValidationContext().staticValidationVisitor());
+          config, validation_visitor);
 
-  auto wasm_service_extension = std::make_unique<WasmServiceExtension>();
-  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<WasmServiceExtension>(typed_config);
 }
 
 // /**

source/extensions/bootstrap/wasm/config.h

@@ -34,37 +34,34 @@ class WasmService {
 };
 
 using WasmServicePtr = std::unique_ptr<WasmService>;
-using CreateWasmServiceCallback = std::function<void(WasmServicePtr)>;
 
-class WasmFactory : public Server::Configuration::BootstrapExtensionFactory,
-                    Logger::Loggable<Logger::Id::wasm> {
+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<envoy::extensions::wasm::v3::WasmService>();
   }
-
-private:
-  Config::DataSource::RemoteAsyncDataProviderPtr remote_data_provider_;
 };
 
-class WasmServiceExtension : public Server::BootstrapExtension {
+class WasmServiceExtension : public Server::BootstrapExtension, Logger::Loggable<Logger::Id::wasm> {
 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

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,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, serverFactoryContext()));
+    bootstrap_extensions_.push_back(factory.createBootstrapExtension(
+        *config, messageValidationContext().staticValidationVisitor()));
   }
 
   // Register the fatal actions.
@@ -534,6 +537,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() ||

test/extensions/bootstrap/wasm/config_test.cc

@@ -52,7 +52,9 @@ class WasmFactoryTest : public testing::TestWithParam<std::string> {
     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<Bootstrap::Wasm::WasmServiceExtension*>(extension_.get())->wasmService();
     EXPECT_CALL(init_watcher_, ready());
     init_manager_.initialize(init_watcher_);

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;
 

test/mocks/server/bootstrap_extension_factory.h

@@ -6,17 +6,28 @@
 
 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:
   MockBootstrapExtensionFactory();
   ~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));
 };
+
 } // namespace Configuration
 } // namespace Server
 } // namespace Envoy

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<const test::common::config::DummyConfig*>(&config);
         EXPECT_NE(nullptr, proto);
         EXPECT_EQ(proto->a(), "foo");
-        return std::make_unique<FooBootstrapExtension>();
+        auto mock_extension = std::make_unique<MockBootstrapExtension>();
+        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<Configuration::BootstrapExtensionFactory> 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.");
       }));